From 6b72099d763243830eafcd9d801ce9cd2d93965f Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Sat, 28 Aug 2021 21:07:28 -0700 Subject: [PATCH] [spv-out] Let write_type_declaration_local handle all LocalTypes. Change `write_type_declaration_local` to handle any type that can be represented as a `LocalType`. This means that Image and Sampler types are handled there now. Change `write_type_declaration_arena` to defer to `write_type_declaration_local` to handle any type that can be represented as a `LocalType`. This makes the code for those types in `write_type_declaration_arena` redundant, so remove it. However, continue to request SPIR-V capabilities for image types from `write_type_declaration_arena`, since that function is fallible, while `write_type_declaration_local` is not. --- src/back/spv/mod.rs | 7 + src/back/spv/writer.rs | 374 ++++++++++++++++++++--------------------- 2 files changed, 190 insertions(+), 191 deletions(-) diff --git a/src/back/spv/mod.rs b/src/back/spv/mod.rs index c26ed6187a..7aeef75828 100644 --- a/src/back/spv/mod.rs +++ b/src/back/spv/mod.rs @@ -161,6 +161,13 @@ impl Function { /// so we can't ever create a `Handle` to refer to them. So for local use /// in the SPIR-V writer, we have this home-grown type enum that covers only the /// cases we need (for example, it doesn't cover structs). +/// +/// As explained in ยง2.8 of the SPIR-V spec, some classes of type instructions +/// must be unique; for example, you can't have two `OpTypeInt 32 1` +/// instructions in the same module. `Writer::lookup_type` maps each `LocalType` +/// value for which we've written instructions to its id, so we can avoid +/// writing out duplicates. `LocalType` also includes variants like `Pointer` +/// that do not need to be unique - but it is harmless to avoid the duplication. #[derive(Debug, PartialEq, Hash, Eq, Copy, Clone)] enum LocalType { /// A scalar, vector, or pointer to one of those. diff --git a/src/back/spv/writer.rs b/src/back/spv/writer.rs index bbb9827f57..036eb4e74b 100644 --- a/src/back/spv/writer.rs +++ b/src/back/spv/writer.rs @@ -591,6 +591,50 @@ impl Writer { } } + fn request_image_capabilities(&mut self, inner: &crate::TypeInner) -> Result<(), Error> { + if let crate::TypeInner::Image { + dim, + arrayed, + class, + } = *inner + { + let sampled = match class { + crate::ImageClass::Sampled { .. } => true, + crate::ImageClass::Depth { .. } => true, + crate::ImageClass::Storage { format, .. } => { + self.request_image_format_capabilities(format.into())?; + false + } + }; + + match dim { + crate::ImageDimension::D1 => { + if sampled { + self.require_any("sampled 1D images", &[spirv::Capability::Sampled1D])?; + } else { + self.require_any("1D storage images", &[spirv::Capability::Image1D])?; + } + } + crate::ImageDimension::Cube if arrayed => { + if sampled { + self.require_any( + "sampled cube array images", + &[spirv::Capability::SampledCubeArray], + )?; + } else { + self.require_any( + "cube array storage images", + &[spirv::Capability::ImageCubeArray], + )?; + } + } + _ => {} + } + } + + Ok(()) + } + fn write_type_declaration_local(&mut self, id: Word, local_ty: LocalType) { let instruction = match local_ty { LocalType::Value { @@ -644,8 +688,27 @@ impl Writer { })); Instruction::type_pointer(id, class, type_id) } - // all the samplers and image types go through `write_type_declaration_arena` - LocalType::Image { .. } | LocalType::Sampler => unreachable!(), + LocalType::Image { + dim, + arrayed, + class, + } => { + let kind = match class { + crate::ImageClass::Sampled { kind, multi: _ } => kind, + crate::ImageClass::Depth { multi: _ } => crate::ScalarKind::Float, + crate::ImageClass::Storage { format, .. } => format.into(), + }; + let local_type = LocalType::Value { + vector_size: None, + kind, + width: 4, + pointer_class: None, + }; + let type_id = self.get_type_id(LookupType::Local(local_type)); + let dim = map_dim(dim); + Instruction::type_image(id, type_id, dim, arrayed, class) + } + LocalType::Sampler => Instruction::type_sampler(id), LocalType::SampledImage { image_type_id } => { Instruction::type_sampled_image(id, image_type_id) } @@ -663,204 +726,133 @@ impl Writer { let decorate_layout = true; //TODO? let id = if let Some(local) = make_local(&ty.inner) { + // This type can be represented as a `LocalType`, so check if we've + // already written an instruction for it. If not, do so now, with + // `write_type_declaration_local`. match self.lookup_type.entry(LookupType::Local(local)) { - // if it's already known as local, re-use it - Entry::Occupied(e) => { - let id = *e.into_mut(); - self.lookup_type.insert(LookupType::Handle(handle), id); - return Ok(id); - } - // also register the type as "local", to avoid duplication + // We already have an id for this `LocalType`. + Entry::Occupied(e) => *e.get(), + + // It's a type we haven't seen before. Entry::Vacant(e) => { let id = self.id_gen.next(); - *e.insert(id) + e.insert(id); + self.write_type_declaration_local(id, local); + + // If it's an image type, request SPIR-V capabilities here, so + // write_type_declaration_local can stay infallible. + self.request_image_capabilities(&ty.inner)?; + + id } } } else { - self.id_gen.next() + use spirv::Decoration; + + let id = self.id_gen.next(); + let instruction = match ty.inner { + crate::TypeInner::Array { base, size, stride } => { + if decorate_layout { + self.decorate(id, Decoration::ArrayStride, &[stride]); + } + + let type_id = self.get_type_id(LookupType::Handle(base)); + match size { + crate::ArraySize::Constant(const_handle) => { + let length_id = self.constant_ids[const_handle.index()]; + Instruction::type_array(id, type_id, length_id) + } + crate::ArraySize::Dynamic => Instruction::type_runtime_array(id, type_id), + } + } + crate::TypeInner::Struct { + top_level, + ref members, + span: _, + } => { + if self.flags.contains(WriterFlags::DEBUG) { + if let Some(ref name) = ty.name { + self.debugs.push(Instruction::name(id, name)); + } + } + + if top_level { + self.decorate(id, Decoration::Block, &[]); + } + + let mut member_ids = Vec::with_capacity(members.len()); + for (index, member) in members.iter().enumerate() { + if decorate_layout { + self.annotations.push(Instruction::member_decorate( + id, + index as u32, + Decoration::Offset, + &[member.offset], + )); + } + + if self.flags.contains(WriterFlags::DEBUG) { + if let Some(ref name) = member.name { + self.debugs + .push(Instruction::member_name(id, index as u32, name)); + } + } + + // The matrix decorations also go on arrays of matrices, + // so lets check this first. + let member_array_subty_inner = match arena[member.ty].inner { + crate::TypeInner::Array { base, .. } => &arena[base].inner, + ref other => other, + }; + if let crate::TypeInner::Matrix { + columns, + rows: _, + width, + } = *member_array_subty_inner + { + let byte_stride = match columns { + crate::VectorSize::Bi => 2 * width, + crate::VectorSize::Tri | crate::VectorSize::Quad => 4 * width, + }; + self.annotations.push(Instruction::member_decorate( + id, + index as u32, + Decoration::ColMajor, + &[], + )); + self.annotations.push(Instruction::member_decorate( + id, + index as u32, + Decoration::MatrixStride, + &[byte_stride as u32], + )); + } + + let member_id = self.get_type_id(LookupType::Handle(member.ty)); + member_ids.push(member_id); + } + Instruction::type_struct(id, member_ids.as_slice()) + } + + // These all have TypeLocal representations, so they should have been + // handled by `write_type_declaration_local` above. + crate::TypeInner::Scalar { .. } + | crate::TypeInner::Atomic { .. } + | crate::TypeInner::Vector { .. } + | crate::TypeInner::Matrix { .. } + | crate::TypeInner::Pointer { .. } + | crate::TypeInner::ValuePointer { .. } + | crate::TypeInner::Image { .. } + | crate::TypeInner::Sampler { .. } => unreachable!(), + }; + + instruction.to_words(&mut self.logical_layout.declarations); + id }; + + // Add this handle as a new alias for that type. self.lookup_type.insert(LookupType::Handle(handle), id); - if self.flags.contains(WriterFlags::DEBUG) { - if let Some(ref name) = ty.name { - self.debugs.push(Instruction::name(id, name)); - } - } - - use spirv::Decoration; - - let instruction = match ty.inner { - crate::TypeInner::Scalar { kind, width } | crate::TypeInner::Atomic { kind, width } => { - self.make_scalar(id, kind, width) - } - crate::TypeInner::Vector { size, kind, width } => { - let scalar_id = self.get_type_id(LookupType::Local(LocalType::Value { - vector_size: None, - kind, - width, - pointer_class: None, - })); - Instruction::type_vector(id, scalar_id, size) - } - crate::TypeInner::Matrix { - columns, - rows, - width, - } => { - let vector_id = self.get_type_id(LookupType::Local(LocalType::Value { - vector_size: Some(rows), - kind: crate::ScalarKind::Float, - width, - pointer_class: None, - })); - Instruction::type_matrix(id, vector_id, columns) - } - crate::TypeInner::Image { - dim, - arrayed, - class, - } => { - let (kind, sampled) = match class { - crate::ImageClass::Sampled { kind, multi: _ } => (kind, true), - crate::ImageClass::Depth { multi: _ } => (crate::ScalarKind::Float, true), - crate::ImageClass::Storage { format, .. } => { - self.request_image_format_capabilities(spirv::ImageFormat::from(format))?; - (crate::ScalarKind::from(format), false) - } - }; - let local_type = LocalType::Value { - vector_size: None, - kind, - width: 4, - pointer_class: None, - }; - let dim = map_dim(dim); - match dim { - spirv::Dim::Dim1D => { - if sampled { - self.require_any("sampled 1D images", &[spirv::Capability::Sampled1D])?; - } else { - self.require_any("1D storage images", &[spirv::Capability::Image1D])?; - } - } - spirv::Dim::DimCube if arrayed => { - if sampled { - self.require_any( - "sampled cube array images", - &[spirv::Capability::SampledCubeArray], - )?; - } else { - self.require_any( - "cube array storage images", - &[spirv::Capability::ImageCubeArray], - )?; - } - } - _ => {} - } - let type_id = self.get_type_id(LookupType::Local(local_type)); - Instruction::type_image(id, type_id, dim, arrayed, class) - } - crate::TypeInner::Sampler { comparison: _ } => Instruction::type_sampler(id), - crate::TypeInner::Array { base, size, stride } => { - if decorate_layout { - self.decorate(id, Decoration::ArrayStride, &[stride]); - } - - let type_id = self.get_type_id(LookupType::Handle(base)); - match size { - crate::ArraySize::Constant(const_handle) => { - let length_id = self.constant_ids[const_handle.index()]; - Instruction::type_array(id, type_id, length_id) - } - crate::ArraySize::Dynamic => Instruction::type_runtime_array(id, type_id), - } - } - crate::TypeInner::Struct { - top_level, - ref members, - span: _, - } => { - if top_level { - self.decorate(id, Decoration::Block, &[]); - } - - let mut member_ids = Vec::with_capacity(members.len()); - for (index, member) in members.iter().enumerate() { - if decorate_layout { - self.annotations.push(Instruction::member_decorate( - id, - index as u32, - Decoration::Offset, - &[member.offset], - )); - } - - if self.flags.contains(WriterFlags::DEBUG) { - if let Some(ref name) = member.name { - self.debugs - .push(Instruction::member_name(id, index as u32, name)); - } - } - - // The matrix decorations also go on arrays of matrices, - // so lets check this first. - let member_array_subty_inner = match arena[member.ty].inner { - crate::TypeInner::Array { base, .. } => &arena[base].inner, - ref other => other, - }; - if let crate::TypeInner::Matrix { - columns, - rows: _, - width, - } = *member_array_subty_inner - { - let byte_stride = match columns { - crate::VectorSize::Bi => 2 * width, - crate::VectorSize::Tri | crate::VectorSize::Quad => 4 * width, - }; - self.annotations.push(Instruction::member_decorate( - id, - index as u32, - Decoration::ColMajor, - &[], - )); - self.annotations.push(Instruction::member_decorate( - id, - index as u32, - Decoration::MatrixStride, - &[byte_stride as u32], - )); - } - - let member_id = self.get_type_id(LookupType::Handle(member.ty)); - member_ids.push(member_id); - } - Instruction::type_struct(id, member_ids.as_slice()) - } - crate::TypeInner::Pointer { base, class } => { - let type_id = self.get_type_id(LookupType::Handle(base)); - let raw_class = map_storage_class(class); - Instruction::type_pointer(id, raw_class, type_id) - } - crate::TypeInner::ValuePointer { - size, - kind, - width, - class, - } => { - let raw_class = map_storage_class(class); - let type_id = self.get_type_id(LookupType::Local(LocalType::Value { - vector_size: size, - kind, - width, - pointer_class: None, - })); - Instruction::type_pointer(id, raw_class, type_id) - } - }; - - instruction.to_words(&mut self.logical_layout.declarations); Ok(id) }