Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deriving SPIR-V capabilities #881

Merged
merged 1 commit into from
May 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/back/msl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2428,7 +2428,7 @@ fn test_stack_size() {
let stack_size = addresses.end - addresses.start;
// check the size (in debug only)
// last observed macOS value: 17664
if stack_size < 17000 || stack_size > 19000 {
if stack_size < 14000 || stack_size > 19000 {
panic!("`put_expression` stack size {} has changed!", stack_size);
}
}
Expand All @@ -2443,7 +2443,7 @@ fn test_stack_size() {
let stack_size = addresses.end - addresses.start;
// check the size (in debug only)
// last observed macOS value: 13600
if stack_size < 12000 || stack_size > 14500 {
if stack_size < 11000 || stack_size > 14500 {
panic!("`put_block` stack size {} has changed!", stack_size);
}
}
Expand Down
63 changes: 39 additions & 24 deletions src/back/spv/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ pub struct Writer {
logical_layout: LogicalLayout,
id_gen: IdGenerator,
capabilities: crate::FastHashSet<spirv::Capability>,
strict_capabilities: bool,
forbidden_caps: Option<&'static [spirv::Capability]>,
debugs: Vec<Instruction>,
annotations: Vec<Instruction>,
flags: WriterFlags,
Expand Down Expand Up @@ -313,19 +313,22 @@ impl Writer {
let gl450_ext_inst_id = id_gen.next();
let void_type = id_gen.next();

let (capabilities, forbidden_caps) = match options.capabilities {
Some(ref caps) => (caps.clone(), None),
None => {
let mut caps = crate::FastHashSet::default();
caps.insert(spirv::Capability::Shader);
let forbidden: &[_] = &[spirv::Capability::Kernel];
(caps, Some(forbidden))
}
};

Ok(Writer {
physical_layout: PhysicalLayout::new(raw_version),
logical_layout: LogicalLayout::default(),
id_gen,
capabilities: match options.capabilities {
Some(ref caps) => caps.clone(),
None => {
let mut caps = crate::FastHashSet::default();
caps.insert(spirv::Capability::Shader);
caps
}
},
strict_capabilities: options.capabilities.is_some(),
capabilities,
forbidden_caps,
debugs: vec![],
annotations: vec![],
flags: options.flags,
Expand All @@ -344,20 +347,21 @@ impl Writer {
}

fn check(&mut self, capabilities: &[spirv::Capability]) -> Result<(), Error> {
if self.strict_capabilities {
if capabilities.is_empty()
|| capabilities
.iter()
.any(|cap| self.capabilities.contains(cap))
{
Ok(())
} else {
Err(Error::MissingCapabilities(capabilities.to_vec()))
if capabilities.is_empty()
|| capabilities
.iter()
.any(|cap| self.capabilities.contains(cap))
{
return Ok(());
}
if let Some(forbidden) = self.forbidden_caps {
// take the first allowed capability, blindly
if let Some(&cap) = capabilities.iter().find(|cap| !forbidden.contains(cap)) {
self.capabilities.insert(cap);
return Ok(());
}
} else {
self.capabilities.extend(capabilities);
Ok(())
}
Err(Error::MissingCapabilities(capabilities.to_vec()))
}

fn get_type_id(
Expand Down Expand Up @@ -887,17 +891,25 @@ impl Writer {
let kind = match class {
crate::ImageClass::Sampled { kind, multi: _ } => kind,
crate::ImageClass::Depth => crate::ScalarKind::Float,
crate::ImageClass::Storage(format) => format.into(),
crate::ImageClass::Storage(format) => {
let required_caps: &[_] = match dim {
crate::ImageDimension::D1 => &[spirv::Capability::Image1D],
crate::ImageDimension::Cube => &[spirv::Capability::ImageCubeArray],
_ => &[],
};
self.check(required_caps)?;
format.into()
}
};
let local_type = LocalType::Value {
vector_size: None,
kind,
width: 4,
pointer_class: None,
};
let type_id = self.get_type_id(arena, LookupType::Local(local_type))?;
let dim = map_dim(dim);
self.check(dim.required_capabilities())?;
let type_id = self.get_type_id(arena, LookupType::Local(local_type))?;
Instruction::type_image(id, type_id, dim, arrayed, class)
}
crate::TypeInner::Sampler { comparison: _ } => Instruction::type_sampler(id),
Expand Down Expand Up @@ -2212,6 +2224,8 @@ impl Writer {
}
};

self.check(&[spirv::Capability::ImageQuery])?;

match query {
Iq::Size { level } => {
let dim_coords = match dim {
Expand Down Expand Up @@ -2248,6 +2262,7 @@ impl Writer {
(spirv::Op::ImageQuerySizeLod, Some(level_id))
}
};

// The ID of the vector returned by SPIR-V, which contains the dimensions
// as well as the layer count.
let id_extended = self.id_gen.next();
Expand Down
1 change: 0 additions & 1 deletion tests/in/access.param.ron
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
(
spv_version: (1, 1),
spv_capabilities: [ Shader, Image1D, Sampled1D ],
spv_debug: true,
spv_adjust_coordinate_space: false,
msl_custom: true,
Expand Down
1 change: 0 additions & 1 deletion tests/in/boids.param.ron
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
(
spv_version: (1, 0),
spv_capabilities: [ Shader ],
spv_debug: true,
spv_adjust_coordinate_space: false,
msl_custom: true,
Expand Down
1 change: 0 additions & 1 deletion tests/in/collatz.param.ron
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
(
spv_version: (1, 0),
spv_capabilities: [ Shader ],
spv_debug: true,
)
1 change: 0 additions & 1 deletion tests/in/control-flow.param.ron
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
(
spv_version: (1, 1),
spv_capabilities: [ Shader ],
)
1 change: 0 additions & 1 deletion tests/in/empty.param.ron
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
(
spv_version: (1, 1),
spv_capabilities: [ Shader ],
)
1 change: 0 additions & 1 deletion tests/in/extra.param.ron
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
(
god_mode: true,
spv_version: (1, 0),
spv_capabilities: [ Shader ],
)
1 change: 0 additions & 1 deletion tests/in/image.param.ron
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
(
spv_version: (1, 1),
spv_capabilities: [ Shader, ImageQuery, Image1D, Sampled1D ],
spv_debug: true,
)
1 change: 0 additions & 1 deletion tests/in/operators.param.ron
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
(
spv_version: (1, 0),
spv_capabilities: [ Shader ],
)
1 change: 0 additions & 1 deletion tests/in/quad-glsl.param.ron
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
(
spv_version: (1, 0),
spv_capabilities: [ Shader ],
spv_debug: true,
spv_adjust_coordinate_space: true,
)
1 change: 0 additions & 1 deletion tests/in/quad.param.ron
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
(
spv_version: (1, 0),
spv_capabilities: [ Shader ],
spv_debug: true,
spv_adjust_coordinate_space: true,
)
1 change: 0 additions & 1 deletion tests/in/shadow.param.ron
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
(
spv_version: (1, 2),
spv_capabilities: [ Shader ],
spv_debug: true,
spv_adjust_coordinate_space: true,
)
1 change: 0 additions & 1 deletion tests/in/skybox.param.ron
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
(
spv_flow_dump_prefix: "",
spv_version: (1, 0),
spv_capabilities: [ Shader ],
spv_debug: false,
spv_adjust_coordinate_space: false,
msl_custom: true,
Expand Down
1 change: 0 additions & 1 deletion tests/in/standard.param.ron
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
(
spv_version: (1, 0),
spv_capabilities: [ Shader ],
spv_adjust_coordinate_space: false,
)
2 changes: 0 additions & 2 deletions tests/out/access.spvasm
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
; Version: 1.1
; Generator: rspirv
; Bound: 49
OpCapability Image1D
OpCapability Shader
OpCapability Sampled1D
OpExtension "SPV_KHR_storage_buffer_storage_class"
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
Expand Down
3 changes: 1 addition & 2 deletions tests/out/image.spvasm
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
; Version: 1.1
; Generator: rspirv
; Bound: 127
OpCapability ImageQuery
OpCapability Image1D
OpCapability Shader
OpCapability Sampled1D
OpCapability ImageQuery
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %43 "main" %40
Expand Down
7 changes: 6 additions & 1 deletion tests/snapshots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ struct Parameters {
#[cfg_attr(not(feature = "spv-out"), allow(dead_code))]
spv_version: (u8, u8),
#[cfg_attr(not(feature = "spv-out"), allow(dead_code))]
#[serde(default)]
spv_capabilities: naga::FastHashSet<spirv::Capability>,
#[cfg_attr(not(feature = "spv-out"), allow(dead_code))]
#[serde(default)]
Expand Down Expand Up @@ -139,7 +140,11 @@ fn check_output_spv(
let options = spv::Options {
lang_version: params.spv_version,
flags,
capabilities: Some(params.spv_capabilities.clone()),
capabilities: if params.spv_capabilities.is_empty() {
None
} else {
Some(params.spv_capabilities.clone())
},
};

let spv = spv::write_vec(module, info, &options).unwrap();
Expand Down