diff --git a/src/back/glsl/mod.rs b/src/back/glsl/mod.rs index b27b6468b7..3674a2076c 100644 --- a/src/back/glsl/mod.rs +++ b/src/back/glsl/mod.rs @@ -448,11 +448,7 @@ impl<'a, W: Write> Writer<'a, W> { // This are always ordered because of the IR is structured in a way that you can't make a // struct without adding all of it's members first for (handle, ty) in self.module.types.iter() { - if let TypeInner::Struct { - block: _, - ref members, - } = ty.inner - { + if let TypeInner::Struct { ref members, .. } = ty.inner { // No needed to write a struct that also should be written as a global variable let is_global_struct = self .module @@ -670,11 +666,12 @@ impl<'a, W: Write> Writer<'a, W> { // glsl has no pointer types so just write types as normal and loads are skipped TypeInner::Pointer { base, .. } => self.write_type(base), TypeInner::Struct { - block: true, + level: crate::StructLevel::Root, ref members, + span: _, } => self.write_struct(true, ty, members), // glsl structs are written as just the struct name if it isn't a block - TypeInner::Struct { block: false, .. } => { + TypeInner::Struct { .. } => { // Get the struct name let name = &self.names[&NameKey::Type(ty)]; write!(self.out, "{}", name)?; @@ -790,10 +787,7 @@ impl<'a, W: Write> Writer<'a, W> { output: bool, ) -> Result<(), Error> { match self.module.types[ty].inner { - crate::TypeInner::Struct { - block: _, - ref members, - } => { + crate::TypeInner::Struct { ref members, .. } => { for member in members { self.write_varying(member.binding.as_ref(), member.ty, output)?; } @@ -918,10 +912,7 @@ impl<'a, W: Write> Writer<'a, W> { write!(self.out, " {}", name)?; write!(self.out, " = ")?; match self.module.types[arg.ty].inner { - crate::TypeInner::Struct { - block: _, - ref members, - } => { + crate::TypeInner::Struct { ref members, .. } => { self.write_type(arg.ty)?; write!(self.out, "(")?; for (index, member) in members.iter().enumerate() { @@ -1338,10 +1329,7 @@ impl<'a, W: Write> Writer<'a, W> { if let Some(ref result) = ep.function.result { let value = value.unwrap(); match self.module.types[result.ty].inner { - crate::TypeInner::Struct { - block: _, - ref members, - } => { + crate::TypeInner::Struct { ref members, .. } => { for (index, member) in members.iter().enumerate() { let varying_name = VaryingName { binding: member.binding.as_ref().unwrap(), diff --git a/src/back/msl/writer.rs b/src/back/msl/writer.rs index c71f23af3c..74d2637c9d 100644 --- a/src/back/msl/writer.rs +++ b/src/back/msl/writer.rs @@ -1000,10 +1000,7 @@ impl Writer { Some(struct_name) => { let result_ty = context.function.result.as_ref().unwrap().ty; match context.module.types[result_ty].inner { - crate::TypeInner::Struct { - block: _, - ref members, - } => { + crate::TypeInner::Struct { ref members, .. } => { let tmp = "_tmp"; write!(self.out, "{}const auto {} = ", level, tmp)?; self.put_expression(expr_handle, context, true)?; @@ -1373,12 +1370,16 @@ impl Writer { } } } - crate::TypeInner::Struct { - block: _, - ref members, - } => { + crate::TypeInner::Struct { ref members, .. } => { writeln!(self.out, "struct {} {{", name)?; + let mut last_offset = 0; for (index, member) in members.iter().enumerate() { + // quick and dirty way to figure out if we need this... + if member.binding.is_none() && member.offset > last_offset { + let pad = member.offset - last_offset; + //TODO: adjust the struct initializers + writeln!(self.out, "{}char _pad{}[{}];", INDENT, index, pad)?; + } let member_name = &self.names[&NameKey::StructMember(handle, index as u32)]; let base_name = TypeContext { handle: member.ty, @@ -1389,13 +1390,16 @@ impl Writer { first_time: false, }; writeln!(self.out, "{}{} {};", INDENT, base_name, member_name)?; - // quick and dirty way to figure out if we need this... - if member.binding.is_none() { - let pad = - member.span - module.types[member.ty].inner.span(&module.constants); - if pad != 0 { - writeln!(self.out, "{}char _pad{}[{}];", INDENT, index, pad)?; - } + let ty_inner = &module.types[member.ty].inner; + last_offset = member.offset + ty_inner.span(&module.constants); + // for 3-component vectors, add one component + if let crate::TypeInner::Vector { + size: crate::VectorSize::Tri, + kind: _, + width, + } = *ty_inner + { + last_offset += width as u32; } } writeln!(self.out, "}};")?; @@ -1728,10 +1732,7 @@ impl Writer { let mut argument_members = Vec::new(); for (arg_index, arg) in fun.arguments.iter().enumerate() { match module.types[arg.ty].inner { - crate::TypeInner::Struct { - block: _, - ref members, - } => { + crate::TypeInner::Struct { ref members, .. } => { for (member_index, member) in members.iter().enumerate() { argument_members.push(( NameKey::StructMember(arg.ty, member_index as u32), @@ -1778,10 +1779,8 @@ impl Writer { let result_type_name = match fun.result { Some(ref result) => { let mut result_members = Vec::new(); - if let crate::TypeInner::Struct { - block: _, - ref members, - } = module.types[result.ty].inner + if let crate::TypeInner::Struct { ref members, .. } = + module.types[result.ty].inner { for (member_index, member) in members.iter().enumerate() { result_members.push(( @@ -1959,10 +1958,7 @@ impl Writer { let arg_name = &self.names[&NameKey::EntryPointArgument(ep_index as _, arg_index as u32)]; match module.types[arg.ty].inner { - crate::TypeInner::Struct { - block: _, - ref members, - } => { + crate::TypeInner::Struct { ref members, .. } => { let struct_name = &self.names[&NameKey::Type(arg.ty)]; write!( self.out, diff --git a/src/back/spv/writer.rs b/src/back/spv/writer.rs index d82a5059ee..aa720a5548 100644 --- a/src/back/spv/writer.rs +++ b/src/back/spv/writer.rs @@ -455,10 +455,8 @@ impl Writer { .body .push(Instruction::load(argument_type_id, id, varying_id, None)); id - } else if let crate::TypeInner::Struct { - block: _, - ref members, - } = ir_module.types[argument.ty].inner + } else if let crate::TypeInner::Struct { ref members, .. } = + ir_module.types[argument.ty].inner { let struct_id = self.id_gen.next(); let mut constituent_ids = Vec::with_capacity(members.len()); @@ -509,10 +507,8 @@ impl Writer { type_id, built_in: binding.to_built_in(), }); - } else if let crate::TypeInner::Struct { - block: _, - ref members, - } = ir_module.types[result.ty].inner + } else if let crate::TypeInner::Struct { ref members, .. } = + ir_module.types[result.ty].inner { for member in members { let type_id = @@ -862,13 +858,16 @@ impl Writer { crate::ArraySize::Dynamic => Instruction::type_runtime_array(id, type_id), } } - crate::TypeInner::Struct { block, ref members } => { - if block { + crate::TypeInner::Struct { + ref level, + ref members, + span: _, + } => { + if let crate::StructLevel::Root = *level { self.annotations .push(Instruction::decorate(id, Decoration::Block, &[])); } - let mut current_offset = 0; let mut member_ids = Vec::with_capacity(members.len()); for (index, member) in members.iter().enumerate() { if decorate_layout { @@ -876,9 +875,8 @@ impl Writer { id, index as u32, Decoration::Offset, - &[current_offset], + &[member.offset], )); - current_offset += member.span; } if self.flags.contains(WriterFlags::DEBUG) { diff --git a/src/front/glsl/functions.rs b/src/front/glsl/functions.rs index e1492e84b8..ed3908fcae 100644 --- a/src/front/glsl/functions.rs +++ b/src/front/glsl/functions.rs @@ -241,11 +241,7 @@ impl Program<'_> { } else { let ty = &self.module.types[var.ty]; // anonymous structs - if let TypeInner::Struct { - block: _, - ref members, - } = ty.inner - { + if let TypeInner::Struct { ref members, .. } = ty.inner { let base = self .context .expressions diff --git a/src/front/glsl/parser.rs b/src/front/glsl/parser.rs index 9f09b27e92..10eacb5e32 100644 --- a/src/front/glsl/parser.rs +++ b/src/front/glsl/parser.rs @@ -514,15 +514,21 @@ pomelo! { if i.1 == "gl_PerVertex" { None } else { - let block = !t.is_empty(); + let level = if t.is_empty() { + //TODO + crate::StructLevel::Normal { alignment: crate::Alignment::new(1).unwrap() } + } else { + crate::StructLevel::Root + }; Some(VarDeclaration { type_qualifiers: t, ids_initializers: vec![(None, None)], ty: extra.module.types.fetch_or_append(Type{ name: Some(i.1), inner: TypeInner::Struct { - block, + level, members: sdl, + span: 0, //TODO }, }), }) @@ -531,15 +537,21 @@ pomelo! { declaration ::= type_qualifier(t) Identifier(i1) LeftBrace struct_declaration_list(sdl) RightBrace Identifier(i2) Semicolon { - let block = !t.is_empty(); + let level = if t.is_empty() { + //TODO + crate::StructLevel::Normal { alignment: crate::Alignment::new(1).unwrap() } + } else { + crate::StructLevel::Root + }; Some(VarDeclaration { type_qualifiers: t, ids_initializers: vec![(Some(i2.1), None)], ty: extra.module.types.fetch_or_append(Type{ name: Some(i1.1), inner: TypeInner::Struct { - block, + level, members: sdl, + span: 0, //TODO }, }), }) @@ -706,8 +718,9 @@ pomelo! { Type{ name: Some(i.1), inner: TypeInner::Struct { - block: false, + level: crate::StructLevel::Normal { alignment: crate::Alignment::new(1).unwrap() }, members: vec![], + span: 0, } } } @@ -729,7 +742,7 @@ pomelo! { binding: None, //TODO //TODO: if the struct is a uniform struct, these values have to reflect // std140 layout. Otherwise, std430. - span: extra.module.types[ty].inner.span(&extra.module.constants), + offset: 0, }).collect() } else { return Err(ErrorKind::SemanticError("Struct member can't be void".into())) diff --git a/src/front/glsl/variables.rs b/src/front/glsl/variables.rs index d08c59347e..1adf259644 100644 --- a/src/front/glsl/variables.rs +++ b/src/front/glsl/variables.rs @@ -133,10 +133,7 @@ impl Program<'_> { meta: TokenMetadata, ) -> Result, ErrorKind> { match *self.resolve_type(expression)? { - TypeInner::Struct { - block: _, - ref members, - } => { + TypeInner::Struct { ref members, .. } => { let index = members .iter() .position(|m| m.name == Some(name.into())) diff --git a/src/front/spv/function.rs b/src/front/spv/function.rs index 82f74f16ac..2a6f83bad7 100644 --- a/src/front/spv/function.rs +++ b/src/front/spv/function.rs @@ -262,7 +262,7 @@ impl> super::Parser { name: None, ty: result.ty, binding: result.binding.clone(), - span: module.types[result.ty].inner.span(&module.constants), + offset: 0, }); // populate just the globals first, then do `Load` in a // separate step, so that we can get a range. @@ -328,8 +328,11 @@ impl> super::Parser { let ty = module.types.append(crate::Type { name: None, inner: crate::TypeInner::Struct { - block: false, + level: crate::StructLevel::Normal { + alignment: crate::Alignment::new(1).unwrap(), + }, members, + span: 0xFFFF, // shouldn't matter }, }); let result_expr = function diff --git a/src/front/spv/mod.rs b/src/front/spv/mod.rs index 8baf6834a7..6a4164665d 100644 --- a/src/front/spv/mod.rs +++ b/src/front/spv/mod.rs @@ -2488,7 +2488,6 @@ impl> Parser { let mut members = Vec::::with_capacity(inst.wc as usize - 2); let mut member_type_ids = Vec::with_capacity(members.capacity()); - let mut last_offset = 0; for i in 0..u32::from(inst.wc) - 2 { let type_id = self.next()?; member_type_ids.push(type_id); @@ -2499,28 +2498,40 @@ impl> Parser { .unwrap_or_default(); let binding = decor.io_binding().ok(); - if let Some(offset) = decor.offset { - if offset != last_offset { - if let Some(member) = members.last_mut() { - // add padding, if necessary - member.span = offset - (last_offset - member.span); + let offset = match decor.offset { + Some(offset) => offset, + None => match members.last() { + Some(member) => { + member.offset + module.types[member.ty].inner.span(&module.constants) } - last_offset = offset; - } - } - let span = module.types[ty].inner.span(&module.constants); - last_offset += span; - + None => 0, + }, + }; members.push(crate::StructMember { name: decor.name, ty, binding, - span, + offset, }); } + //TODO: we should be able to do better than this. + const STRUCT_ALIGNMENT: u32 = 16; + let inner = crate::TypeInner::Struct { - block: block_decor.is_some(), + level: match block_decor { + Some(_) => crate::StructLevel::Root, + None => crate::StructLevel::Normal { + alignment: crate::Alignment::new(STRUCT_ALIGNMENT).unwrap(), + }, + }, + span: match members.last() { + Some(member) => { + let end = member.offset + module.types[member.ty].inner.span(&module.constants); + ((end - 1) | (STRUCT_ALIGNMENT - 1)) + 1 + } + None => STRUCT_ALIGNMENT, + }, members, }; let ty_handle = module.types.append(crate::Type { diff --git a/src/front/wgsl/layout.rs b/src/front/wgsl/layout.rs index 91734067f0..e17d5548e1 100644 --- a/src/front/wgsl/layout.rs +++ b/src/front/wgsl/layout.rs @@ -1,7 +1,5 @@ use crate::arena::{Arena, Handle}; -use std::num::NonZeroU32; - -pub type Alignment = NonZeroU32; +use std::{num::NonZeroU32, ops}; /// Alignment information for a type. #[derive(Clone, Copy, Debug, Hash, PartialEq)] @@ -9,7 +7,7 @@ pub type Alignment = NonZeroU32; #[cfg_attr(feature = "deserialize", derive(serde::Deserialize))] pub struct TypeLayout { pub size: u32, - pub alignment: Alignment, + pub alignment: crate::Alignment, } /// Helper processor that derives the sizes of all types. @@ -22,17 +20,12 @@ pub struct Layouter { layouts: Vec, } -pub struct Placement { - pub pad: crate::Span, - pub span: crate::Span, -} - impl Layouter { pub fn clear(&mut self) { self.layouts.clear(); } - pub fn round_up(alignment: Alignment, offset: u32) -> u32 { + pub fn round_up(alignment: crate::Alignment, offset: u32) -> u32 { match offset & (alignment.get() - 1) { 0 => offset, other => offset + alignment.get() - other, @@ -43,9 +36,9 @@ impl Layouter { &self, offset: u32, ty: Handle, - align: Option, + align: Option, size: Option, - ) -> Placement { + ) -> (ops::Range, crate::Alignment) { let layout = self.layouts[ty.index()]; let alignment = align.unwrap_or(layout.alignment); let start = Self::round_up(alignment, offset); @@ -53,10 +46,7 @@ impl Layouter { Some(size) => size.get(), None => layout.size, }; - Placement { - pad: start - offset, - span, - } + (start..start + span, alignment) } pub fn update(&mut self, types: &Arena, constants: &Arena) { @@ -66,7 +56,7 @@ impl Layouter { let layout = match ty.inner { Ti::Scalar { width, .. } => TypeLayout { size, - alignment: Alignment::new(width as u32).unwrap(), + alignment: crate::Alignment::new(width as u32).unwrap(), }, Ti::Vector { size: vec_size, @@ -80,7 +70,7 @@ impl Layouter { } else { 2 }; - Alignment::new((count * width) as u32).unwrap() + crate::Alignment::new((count * width) as u32).unwrap() }, }, Ti::Matrix { @@ -91,36 +81,31 @@ impl Layouter { size, alignment: { let count = if rows >= crate::VectorSize::Tri { 4 } else { 2 }; - Alignment::new((count * width) as u32).unwrap() + crate::Alignment::new((count * width) as u32).unwrap() }, }, Ti::Pointer { .. } | Ti::ValuePointer { .. } => TypeLayout { size, - alignment: Alignment::new(1).unwrap(), + alignment: crate::Alignment::new(1).unwrap(), }, Ti::Array { stride, .. } => TypeLayout { size, - alignment: Alignment::new(stride).unwrap(), + alignment: crate::Alignment::new(stride).unwrap(), }, Ti::Struct { - block: _, - ref members, - } => { - let mut total = 0; - let mut biggest_alignment = Alignment::new(1).unwrap(); - for member in members { - let layout = self.layouts[member.ty.index()]; - biggest_alignment = biggest_alignment.max(layout.alignment); - total += member.span; - } - TypeLayout { - size: Self::round_up(biggest_alignment, total), - alignment: biggest_alignment, - } - } + ref level, + members: _, + span, + } => TypeLayout { + size: span, + alignment: match *level { + crate::StructLevel::Root => crate::Alignment::new(1).unwrap(), + crate::StructLevel::Normal { alignment } => alignment, + }, + }, Ti::Image { .. } | Ti::Sampler { .. } => TypeLayout { size, - alignment: Alignment::new(1).unwrap(), + alignment: crate::Alignment::new(1).unwrap(), }, }; debug_assert!(ty.inner.span(constants) <= layout.size); diff --git a/src/front/wgsl/mod.rs b/src/front/wgsl/mod.rs index f4c19e00f7..e618d68124 100644 --- a/src/front/wgsl/mod.rs +++ b/src/front/wgsl/mod.rs @@ -1509,8 +1509,9 @@ impl Parser { lexer: &mut Lexer<'a>, type_arena: &mut Arena, const_arena: &mut Arena, - ) -> Result, Error<'a>> { + ) -> Result<(Vec, u32, crate::Alignment), Error<'a>> { let mut offset = 0; + let mut alignment = crate::Alignment::new(1).unwrap(); let mut members = Vec::new(); lexer.expect(Token::Paren('{'))?; @@ -1556,7 +1557,7 @@ impl Parser { let name = match lexer.next() { (Token::Word(word), _) => word, - (Token::Paren('}'), _) => return Ok(members), + (Token::Paren('}'), _) => return Ok((members, offset, alignment)), other => return Err(Error::Unexpected(other, "field name")), }; lexer.expect(Token::Separator(':'))?; @@ -1564,17 +1565,15 @@ impl Parser { lexer.expect(Token::Separator(';'))?; self.layouter.update(type_arena, const_arena); - let placement = self.layouter.member_placement(offset, ty, align, size); - if placement.pad != 0 { - members.last_mut().unwrap().span += placement.pad; - } - offset += placement.pad + placement.span; + let (range, align) = self.layouter.member_placement(offset, ty, align, size); + alignment = alignment.max(align); + offset = range.end; members.push(crate::StructMember { name: Some(name.to_owned()), ty, binding: bind_parser.finish()?, - span: placement.span, + offset: range.start, }); } } @@ -2556,13 +2555,18 @@ impl Parser { (Token::Separator(';'), _) => {} (Token::Word("struct"), _) => { let name = lexer.next_ident()?; - let members = + let (members, span, alignment) = self.parse_struct_body(lexer, &mut module.types, &mut module.constants)?; let ty = module.types.fetch_or_append(crate::Type { name: Some(name.to_string()), inner: crate::TypeInner::Struct { - block: is_block, + level: if is_block { + crate::StructLevel::Root + } else { + crate::StructLevel::Normal { alignment } + }, members, + span, }, }); self.lookup_type.insert(name.to_owned(), ty); diff --git a/src/lib.rs b/src/lib.rs index d8ab525e4e..fa12613fbc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,6 +50,7 @@ pub use crate::arena::{Arena, Handle, Range}; use std::{ collections::{HashMap, HashSet}, hash::BuildHasherDefault, + num::NonZeroU32, }; #[cfg(feature = "deserialize")] @@ -167,8 +168,7 @@ pub enum BuiltIn { /// Number of bytes per scalar. pub type Bytes = u8; -/// Number of bytes per complex type. -pub type Span = u32; +pub type Alignment = NonZeroU32; /// Number of components in a vector. #[repr(u8)] @@ -245,8 +245,8 @@ pub struct StructMember { pub ty: Handle, /// For I/O structs, defines the binding. pub binding: Option, - /// Size occupied by the member. - pub span: Span, + /// Offset from the beginning from the struct. + pub offset: u32, } /// The number of dimensions an image has. @@ -345,6 +345,18 @@ pub enum ImageClass { Storage(StorageFormat), } +/// Qualifier of the type level, at which a struct can be used. +#[derive(Clone, Copy, Debug, Hash, Eq, Ord, PartialEq, PartialOrd)] +#[cfg_attr(feature = "serialize", derive(Serialize))] +#[cfg_attr(feature = "deserialize", derive(Deserialize))] +pub enum StructLevel { + /// This is a root level struct, it can't be nested inside + /// other composite types. + Root, + /// This is a normal struct, and it has to be aligned for nesting. + Normal { alignment: Alignment }, +} + /// A data type declared in the module. #[derive(Debug, PartialEq)] #[cfg_attr(feature = "serialize", derive(Serialize))] @@ -391,13 +403,13 @@ pub enum TypeInner { Array { base: Handle, size: ArraySize, - stride: Span, + stride: u32, }, /// User-defined structure. Struct { - /// This is a top-level host-shareable structure. - block: bool, + level: StructLevel, members: Vec, + span: u32, }, /// Possibly multidimensional array of texels. Image { diff --git a/src/proc/mod.rs b/src/proc/mod.rs index 1d9e7d8783..55266d2bd2 100644 --- a/src/proc/mod.rs +++ b/src/proc/mod.rs @@ -79,20 +79,12 @@ impl super::TypeInner { size, kind: _, width, - } => { - //let count = if size >= super::VectorSize::Tri { 4 } else { 2 }; - let count = size as u8; //TEMP - (count * width) as u32 - } + } => (size as u8 * width) as u32, Self::Matrix { columns, rows, width, - } => { - //let count = if rows >= super::VectorSize::Tri { 4 } else { 2 }; - let count = rows as u8; //TEMP - (columns as u8 * count * width) as u32 - } + } => (columns as u8 * rows as u8 * width) as u32, Self::Pointer { .. } | Self::ValuePointer { .. } => POINTER_SPAN, Self::Array { base: _, @@ -108,10 +100,7 @@ impl super::TypeInner { }; count * stride } - Self::Struct { - block: _, - ref members, - } => members.iter().map(|m| m.span).sum(), + Self::Struct { span, .. } => span, Self::Image { .. } | Self::Sampler { .. } => 0, } } diff --git a/src/proc/namer.rs b/src/proc/namer.rs index 87ef357ac0..7f62ff1701 100644 --- a/src/proc/namer.rs +++ b/src/proc/namer.rs @@ -77,11 +77,7 @@ impl Namer { let ty_name = self.call_or(&ty.name, "type"); output.insert(NameKey::Type(ty_handle), ty_name); - if let crate::TypeInner::Struct { - block: _, - ref members, - } = ty.inner - { + if let crate::TypeInner::Struct { ref members, .. } = ty.inner { for (index, member) in members.iter().enumerate() { let name = self.call_or(&member.name, "member"); output.insert(NameKey::StructMember(ty_handle, index as u32), name); diff --git a/src/proc/typifier.rs b/src/proc/typifier.rs index 3183c5bd9e..50455defca 100644 --- a/src/proc/typifier.rs +++ b/src/proc/typifier.rs @@ -178,10 +178,7 @@ impl<'a> ResolveContext<'a> { }) } Ti::Array { base, .. } => TypeResolution::Handle(base), - Ti::Struct { - block: _, - ref members, - } => { + Ti::Struct { ref members, .. } => { let member = members .get(index as usize) .ok_or(ResolveError::OutOfBoundsIndex { expr: base, index })?; @@ -234,10 +231,7 @@ impl<'a> ResolveContext<'a> { class, } } - Ti::Struct { - block: _, - ref members, - } => { + Ti::Struct { ref members, .. } => { let member = members .get(index as usize) .ok_or(ResolveError::OutOfBoundsIndex { expr: base, index })?; diff --git a/src/valid/expression.rs b/src/valid/expression.rs index b21f534886..23e6c95467 100644 --- a/src/valid/expression.rs +++ b/src/valid/expression.rs @@ -179,10 +179,7 @@ impl super::Validator { } => module.constants[handle].to_array_length().unwrap(), Ti::Array { .. } => !0, // can't statically know, but need run-time checks Ti::Pointer { .. } => !0, //TODO - Ti::Struct { - ref members, - block: _, - } => members.len() as u32, + Ti::Struct { ref members, .. } => members.len() as u32, ref other => { log::error!("Indexing of {:?}", other); return Err(ExpressionError::InvalidBaseType(base)); @@ -289,10 +286,7 @@ impl super::Validator { } } } - Ti::Struct { - block: _, - ref members, - } => { + Ti::Struct { ref members, .. } => { for (index, (member, &comp)) in members.iter().zip(components).enumerate() { let tin = resolver.resolve(comp)?; if tin != &module.types[member.ty].inner { diff --git a/src/valid/interface.rs b/src/valid/interface.rs index 0d144150c5..17629d1e08 100644 --- a/src/valid/interface.rs +++ b/src/valid/interface.rs @@ -239,8 +239,9 @@ impl VaryingContext<'_> { match self.types[self.ty].inner { //TODO: check the member types crate::TypeInner::Struct { - block: false, + level: crate::StructLevel::Normal { .. }, ref members, + .. } => { for (index, member) in members.iter().enumerate() { self.ty = member.ty; diff --git a/src/valid/type.rs b/src/valid/type.rs index d2cad1f868..ff34e81fe8 100644 --- a/src/valid/type.rs +++ b/src/valid/type.rs @@ -1,7 +1,5 @@ use crate::arena::{Arena, Handle}; -pub type Alignment = u32; - bitflags::bitflags! { #[repr(transparent)] pub struct TypeFlags: u8 { @@ -22,10 +20,16 @@ bitflags::bitflags! { pub enum Disalignment { #[error("The array stride {stride} is not a multiple of the required alignment {alignment}")] ArrayStride { stride: u32, alignment: u32 }, - #[error("The struct size {size}, is not a multiple of the required alignment {alignment}")] - StructSize { size: u32, alignment: u32 }, + #[error("The struct span {span}, is not a multiple of the required alignment {alignment}")] + StructSpan { span: u32, alignment: u32 }, + #[error("The struct span {alignment}, is not a multiple of the member[{member_index}] alignment {member_alignment}")] + StructAlignment { + alignment: u32, + member_index: u32, + member_alignment: u32, + }, #[error("The struct member[{index}] offset {offset} is not a multiple of the required alignment {alignment}")] - Member { + MemberOffset { index: u32, offset: u32, alignment: u32, @@ -52,18 +56,62 @@ pub enum TypeError { InsufficientArrayStride { stride: u32, base_size: u32 }, #[error("Field '{0}' can't be dynamically-sized, has type {1:?}")] InvalidDynamicArray(String, Handle), - #[error("Structure member[{index}] size {size} is not a sufficient to hold {base_size}")] - InsufficientMemberSize { - index: u32, - size: u32, - base_size: u32, - }, + #[error("Structure member[{index}] at {offset} overlaps the previous member")] + MemberOverlap { index: u32, offset: u32 }, + #[error( + "Structure member[{index}] at {offset} and size {size} crosses the structure boundary" + )] + MemberOutOfBounds { index: u32, offset: u32, size: u32 }, #[error("The composite type contains a block structure")] NestedBlock, } // Only makes sense if `flags.contains(HOST_SHARED)` -type LayoutCompatibility = Result, Disalignment)>; +type LayoutCompatibility = Result, (Handle, Disalignment)>; + +fn check_member_layout( + accum: &mut LayoutCompatibility, + member: &crate::StructMember, + member_index: u32, + member_layout: LayoutCompatibility, + struct_level: crate::StructLevel, + ty_handle: Handle, +) { + *accum = match (*accum, member_layout) { + (Ok(cur_alignment), Ok(align)) => { + let align = align.unwrap().get(); + if member.offset % align != 0 { + Err(( + ty_handle, + Disalignment::MemberOffset { + index: member_index, + offset: member.offset, + alignment: align, + }, + )) + } else { + match struct_level { + crate::StructLevel::Normal { alignment } if alignment.get() % align != 0 => { + Err(( + ty_handle, + Disalignment::StructAlignment { + alignment: alignment.get(), + member_index, + member_alignment: align, + }, + )) + } + _ => { + let combined_alignment = + ((cur_alignment.unwrap().get() - 1) | (align - 1)) + 1; + Ok(crate::Alignment::new(combined_alignment)) + } + } + } + } + (Err(e), _) | (_, Err(e)) => Err(e), + }; +} // For the uniform buffer alignment, array strides and struct sizes must be multiples of 16. const UNIFORM_LAYOUT_ALIGNMENT_MASK: u32 = 0xF; @@ -79,12 +127,13 @@ impl TypeInfo { fn dummy() -> Self { TypeInfo { flags: TypeFlags::empty(), - uniform_layout: Ok(0), - storage_layout: Ok(0), + uniform_layout: Ok(None), + storage_layout: Ok(None), } } - fn new(flags: TypeFlags, alignment: crate::Span) -> Self { + fn new(flags: TypeFlags, align: u32) -> Self { + let alignment = crate::Alignment::new(align); TypeInfo { flags, uniform_layout: Ok(alignment), @@ -193,21 +242,36 @@ impl super::Validator { let uniform_layout = match base_info.uniform_layout { Ok(base_alignment) => { // combine the alignment requirements - let alignment = ((base_alignment - 1) | UNIFORM_LAYOUT_ALIGNMENT_MASK) + 1; - if stride % alignment != 0 { - Err((handle, Disalignment::ArrayStride { stride, alignment })) + let align = ((base_alignment.unwrap().get() - 1) + | UNIFORM_LAYOUT_ALIGNMENT_MASK) + + 1; + if stride % align != 0 { + Err(( + handle, + Disalignment::ArrayStride { + stride, + alignment: align, + }, + )) } else { - Ok(alignment) + Ok(crate::Alignment::new(align)) } } Err(e) => Err(e), }; let storage_layout = match base_info.storage_layout { - Ok(alignment) => { - if stride % alignment != 0 { - Err((handle, Disalignment::ArrayStride { stride, alignment })) + Ok(base_alignment) => { + let align = base_alignment.unwrap().get(); + if stride % align != 0 { + Err(( + handle, + Disalignment::ArrayStride { + stride, + alignment: align, + }, + )) } else { - Ok(alignment) + Ok(base_alignment) } } Err(e) => Err(e), @@ -255,14 +319,19 @@ impl super::Validator { storage_layout, } } - Ti::Struct { block, ref members } => { - let mut flags = TypeFlags::DATA - | TypeFlags::SIZED - | TypeFlags::HOST_SHARED - | TypeFlags::INTERFACE; - let mut uniform_layout = Ok(1); - let mut storage_layout = Ok(1); - let mut offset = 0; + Ti::Struct { + level, + ref members, + span, + } => { + let mut ti = TypeInfo::new( + TypeFlags::DATA + | TypeFlags::SIZED + | TypeFlags::HOST_SHARED + | TypeFlags::INTERFACE, + 1, + ); + let mut min_offset = 0; for (i, member) in members.iter().enumerate() { if member.ty >= handle { return Err(TypeError::UnresolvedBase(member.ty)); @@ -271,62 +340,55 @@ impl super::Validator { if !base_info.flags.contains(TypeFlags::DATA) { return Err(TypeError::InvalidData(member.ty)); } - if block && !base_info.flags.contains(TypeFlags::INTERFACE) { + if level == crate::StructLevel::Root + && !base_info.flags.contains(TypeFlags::INTERFACE) + { return Err(TypeError::InvalidBlockType(member.ty)); } if base_info.flags.contains(TypeFlags::BLOCK) { return Err(TypeError::NestedBlock); } - flags &= base_info.flags; + ti.flags &= base_info.flags; + if member.offset < min_offset { + //HACK: this could be nicer. We want to allow some structures + // to not bother with offsets/alignments if they are never + // used for host sharing. + if member.offset == 0 { + ti.flags.set(TypeFlags::HOST_SHARED, false); + } else { + return Err(TypeError::MemberOverlap { + index: i as u32, + offset: member.offset, + }); + } + } let base_size = types[member.ty].inner.span(constants); - if member.span < base_size { - return Err(TypeError::InsufficientMemberSize { + min_offset = member.offset + base_size; + if min_offset > span { + return Err(TypeError::MemberOutOfBounds { index: i as u32, - size: member.span, - base_size, + offset: member.offset, + size: base_size, }); } - uniform_layout = match (uniform_layout, base_info.uniform_layout) { - (Ok(cur_alignment), Ok(alignment)) => { - if offset % alignment != 0 { - Err(( - handle, - Disalignment::Member { - index: i as u32, - offset, - alignment, - }, - )) - } else { - let combined_alignment = - ((cur_alignment - 1) | (alignment - 1)) + 1; - Ok(combined_alignment) - } - } - (Err(e), _) | (_, Err(e)) => Err(e), - }; - storage_layout = match (storage_layout, base_info.storage_layout) { - (Ok(cur_alignment), Ok(alignment)) => { - if offset % alignment != 0 { - Err(( - handle, - Disalignment::Member { - index: i as u32, - offset, - alignment, - }, - )) - } else { - let combined_alignment = - ((cur_alignment - 1) | (alignment - 1)) + 1; - Ok(combined_alignment) - } - } - (Err(e), _) | (_, Err(e)) => Err(e), - }; - offset += member.span; + check_member_layout( + &mut ti.uniform_layout, + member, + i as u32, + base_info.uniform_layout, + level, + handle, + ); + check_member_layout( + &mut ti.storage_layout, + member, + i as u32, + base_info.storage_layout, + level, + handle, + ); // only the last field can be unsized if !base_info.flags.contains(TypeFlags::SIZED) { @@ -334,35 +396,31 @@ impl super::Validator { let name = member.name.clone().unwrap_or_default(); return Err(TypeError::InvalidDynamicArray(name, member.ty)); } - if uniform_layout.is_ok() { - uniform_layout = + if ti.uniform_layout.is_ok() { + ti.uniform_layout = Err((handle, Disalignment::UnsizedMember { index: i as u32 })); } } } - if block { - flags |= TypeFlags::BLOCK; + if let crate::StructLevel::Root = level { + ti.flags |= TypeFlags::BLOCK; } // disabled temporarily, see https://github.com/gpuweb/gpuweb/issues/1558 const CHECK_STRUCT_SIZE: bool = false; if CHECK_STRUCT_SIZE - && uniform_layout.is_ok() - && offset & UNIFORM_LAYOUT_ALIGNMENT_MASK != 0 + && ti.uniform_layout.is_ok() + && span & UNIFORM_LAYOUT_ALIGNMENT_MASK != 0 { - uniform_layout = Err(( + ti.uniform_layout = Err(( handle, - Disalignment::StructSize { - size: offset, + Disalignment::StructSpan { + span, alignment: UNIFORM_LAYOUT_ALIGNMENT_MASK + 1, }, )); } - TypeInfo { - flags, - uniform_layout, - storage_layout, - } + ti } Ti::Image { .. } | Ti::Sampler { .. } => TypeInfo::new(TypeFlags::empty(), 0), }) diff --git a/tests/out/collatz.ron.snap b/tests/out/collatz.ron.snap index 52b22cebe8..b86183c222 100644 --- a/tests/out/collatz.ron.snap +++ b/tests/out/collatz.ron.snap @@ -22,15 +22,16 @@ expression: output ( name: Some("PrimeIndices"), inner: Struct( - block: true, + level: Root, members: [ ( name: Some("data"), ty: 2, binding: None, - span: 4, + offset: 0, ), ], + span: 4, ), ), ( diff --git a/tests/out/shadow.ron.snap b/tests/out/shadow.ron.snap index 1f73006de6..0d57ef42e2 100644 --- a/tests/out/shadow.ron.snap +++ b/tests/out/shadow.ron.snap @@ -106,15 +106,16 @@ expression: output ( name: Some("Globals"), inner: Struct( - block: true, + level: Root, members: [ ( name: Some("num_lights"), ty: 13, binding: None, - span: 16, + offset: 0, ), ], + span: 16, ), ), ( @@ -149,27 +150,30 @@ expression: output ( name: Some("Light"), inner: Struct( - block: false, + level: Normal( + alignment: 16, + ), members: [ ( name: Some("proj"), ty: 18, binding: None, - span: 64, + offset: 0, ), ( name: Some("pos"), ty: 4, binding: None, - span: 16, + offset: 64, ), ( name: Some("color"), ty: 4, binding: None, - span: 16, + offset: 80, ), ], + span: 96, ), ), ( @@ -183,15 +187,16 @@ expression: output ( name: Some("Lights"), inner: Struct( - block: true, + level: Root, members: [ ( name: Some("data"), ty: 20, binding: None, - span: 96, + offset: 0, ), ], + span: 96, ), ), (