wgsl: remove stride

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Change Log
 
+## v0.9 (TBD)
+  - WGSL:
+    - attributes are declared with `@attrib` instead of `[[attrib]]`
+    - `stride` attribute is removed
+    - block comments are supported
+
 ## v0.8 (2021-12-18)
   - development release for wgpu-0.12
   - lots of fixes in all parts

src/back/wgsl/writer.rs

@@ -17,7 +17,6 @@ enum Attribute {
     Interpolate(Option<crate::Interpolation>, Option<crate::Sampling>),
     Location(u32),
     Stage(ShaderStage),
-    Stride(u32),
     WorkGroupSize([u32; 3]),
 }
 
@@ -349,7 +348,6 @@ impl<W: Write> Writer<W> {
                     };
                     write!(self.out, "@stage({}) ", stage_str)?;
                 }
-                Attribute::Stride(stride) => write!(self.out, "@stride({}) ", stride)?,
                 Attribute::WorkGroupSize(size) => {
                     write!(
                         self.out,
@@ -420,15 +418,6 @@ impl<W: Write> Writer<W> {
             // Write struct member name and type
             let member_name = &self.names[&NameKey::StructMember(handle, index as u32)];
             write!(self.out, "{}: ", member_name)?;
-            // Write stride attribute for array struct member
-            if let TypeInner::Array {
-                base: _,
-                size: _,
-                stride,
-            } = module.types[member.ty].inner
-            {
-                self.write_attributes(&[Attribute::Stride(stride)])?;
-            }
             self.write_type(module, member.ty)?;
             write!(self.out, ";")?;
             writeln!(self.out)?;
@@ -523,7 +512,11 @@ impl<W: Write> Writer<W> {
             TypeInner::Atomic { kind, .. } => {
                 write!(self.out, "atomic<{}>", scalar_kind_str(kind))?;
             }
-            TypeInner::Array { base, size, .. } => {
+            TypeInner::Array {
+                base,
+                size,
+                stride: _,
+            } => {
                 // More info https://gpuweb.github.io/gpuweb/wgsl/#array-types
                 // array<A, 3> -- Constant array
                 // array<A> -- Dynamic array

src/front/glsl/mod.rs

@@ -10,7 +10,7 @@ pub use ast::{Precision, Profile};
 pub use error::{Error, ErrorKind, ExpectedToken};
 pub use token::TokenValue;
 
-use crate::{FastHashMap, FastHashSet, Handle, Module, ShaderStage, Span, Type};
+use crate::{proc::Layouter, FastHashMap, FastHashSet, Handle, Module, ShaderStage, Span, Type};
 use ast::{EntryArg, FunctionDeclaration, GlobalLookup};
 use parser::ParsingContext;
 
@@ -166,6 +166,8 @@ pub struct Parser {
 
     entry_args: Vec<EntryArg>,
 
+    layouter: Layouter,
+
     errors: Vec<Error>,
 
     module: Module,
@@ -179,6 +181,7 @@ impl Parser {
         self.lookup_type.clear();
         self.global_variables.clear();
         self.entry_args.clear();
+        self.layouter.clear();
 
         // This is necessary because if the last parsing errored out, the module
         // wouldn't have been swapped

src/front/glsl/types.rs

@@ -259,18 +259,22 @@ impl Parser {
         mut meta: Span,
         array_specifier: Option<(ArraySize, Span)>,
     ) -> Handle<Type> {
-        array_specifier
-            .map(|(size, size_meta)| {
+        match array_specifier {
+            Some((size, size_meta)) => {
                 meta.subsume(size_meta);
-                let stride = self.module.types[base].inner.size(&self.module.constants);
+                self.layouter
+                    .update(&self.module.types, &self.module.constants)
+                    .unwrap();
+                let stride = self.layouter[base].to_stride();
                 self.module.types.insert(
                     Type {
                         name: None,
                         inner: TypeInner::Array { base, size, stride },
                     },
                     meta,
                 )
-            })
-            .unwrap_or(base)
+            }
+            None => base,
+        }
     }
 }

src/front/wgsl/mod.rs

@@ -158,7 +158,6 @@ pub enum Error<'a> {
     UnknownType(Span),
     UnknownStorageFormat(Span),
     UnknownConservativeDepth(Span),
-    ZeroStride(Span),
     ZeroSizeOrAlign(Span),
     InconsistentBinding(Span),
     UnknownLocalFunction(Span),
@@ -232,7 +231,7 @@ impl<'a> Error<'a> {
                         ExpectedToken::Constant => "constant".to_string(),
                         ExpectedToken::PrimaryExpression => "expression".to_string(),
                         ExpectedToken::FieldName => "field name or a closing curly bracket to signify the end of the struct".to_string(),
-                        ExpectedToken::TypeAttribute => "type attribute ('stride')".to_string(),
+                        ExpectedToken::TypeAttribute => "type attribute".to_string(),
                         ExpectedToken::Statement => "statement".to_string(),
                         ExpectedToken::SwitchItem => "switch item ('case' or 'default') or a closing curly bracket to signify the end of the switch statement ('}')".to_string(),
                         ExpectedToken::WorkgroupSizeSeparator => "workgroup size separator (',') or a closing parenthesis".to_string(),
@@ -393,11 +392,6 @@ impl<'a> Error<'a> {
                 labels: vec![(bad_span.clone(), "unknown type".into())],
                 notes: vec![],
             },
-            Error::ZeroStride(ref bad_span) => ParseError {
-                message: "array stride must not be zero".to_string(),
-                labels: vec![(bad_span.clone(), "array stride must not be zero".into())],
-                notes: vec![],
-            },
             Error::ZeroSizeOrAlign(ref bad_span) => ParseError {
                 message: "struct member size or alignment must not be 0".to_string(),
                 labels: vec![(bad_span.clone(), "struct member size or alignment must not be 0".into())],
@@ -1048,7 +1042,9 @@ impl Composition {
 
 #[derive(Default)]
 struct TypeAttributes {
-    stride: Option<NonZeroU32>,
+    // Although WGSL nas no type attributes at the moment, it had them in the past
+// (`[[stride]]`) and may as well acquire some again in the future.
+// Therefore, we are leaving the plumbing in for now.
 }
 
 #[derive(Clone, Debug, PartialEq)]
@@ -2888,7 +2884,7 @@ impl Parser {
     fn parse_type_decl_impl<'a>(
         &mut self,
         lexer: &mut Lexer<'a>,
-        attribute: TypeAttributes,
+        _attribute: TypeAttributes,
         word: &'a str,
         type_arena: &mut UniqueArena<crate::Type>,
         const_arena: &mut Arena<crate::Constant>,
@@ -3024,15 +3020,11 @@ impl Parser {
                     crate::ArraySize::Dynamic
                 };
                 lexer.expect_generic_paren('>')?;
-                let stride = match attribute.stride {
-                    Some(stride) => stride.get(),
-                    None => {
-                        self.layouter.update(type_arena, const_arena).unwrap();
-                        let layout = &self.layouter[base];
-                        Layouter::round_up(layout.alignment, layout.size)
-                    }
-                };
 
+                let stride = {
+                    self.layouter.update(type_arena, const_arena).unwrap();
+                    self.layouter[base].to_stride()
+                };
                 crate::TypeInner::Array { base, size, stride }
             }
             "sampler" => crate::TypeInner::Sampler { comparison: false },
@@ -3240,22 +3232,11 @@ impl Parser {
         const_arena: &mut Arena<crate::Constant>,
     ) -> Result<(Handle<crate::Type>, crate::StorageAccess), Error<'a>> {
         self.push_scope(Scope::TypeDecl, lexer);
-        let mut attribute = TypeAttributes::default();
+        let attribute = TypeAttributes::default();
 
-        while lexer.skip(Token::Attribute) {
-            self.push_scope(Scope::Attribute, lexer);
-            match lexer.next() {
-                (Token::Word("stride"), _) => {
-                    lexer.expect(Token::Paren('('))?;
-                    let (stride, span) =
-                        lexer.capture_span(|lexer| parse_non_negative_sint_literal(lexer, 4))?;
-                    attribute.stride =
-                        Some(NonZeroU32::new(stride).ok_or(Error::ZeroStride(span))?);
-                    lexer.expect(Token::Paren(')'))?;
-                }
-                other => return Err(Error::Unexpected(other, ExpectedToken::TypeAttribute)),
-            }
-            self.pop_scope(lexer);
+        if lexer.skip(Token::Attribute) {
+            let other = lexer.next();
+            return Err(Error::Unexpected(other, ExpectedToken::TypeAttribute));
         }
 
         let storage_access = crate::StorageAccess::default();

src/front/wgsl/tests.rs

@@ -398,7 +398,7 @@ fn parse_array_length() {
     parse_str(
         "
         struct Foo {
-            data: @stride(4) array<u32>;
+            data: array<u32>;
         }; // this is used as both input and output for convenience
 
         @group(0) @binding(0)

src/proc/layouter.rs

@@ -12,6 +12,13 @@ pub struct TypeLayout {
     pub alignment: Alignment,
 }
 
+impl TypeLayout {
+    /// Produce the stride as if this type is a base of an array.
+    pub fn to_stride(&self) -> u32 {
+        Layouter::round_up(self.alignment, self.size)
+    }
+}
+
 /// Helper processor that derives the sizes of all types.
 /// It uses the default layout algorithm/table, described in
 /// <https://github.com/gpuweb/gpuweb/issues/1393>

src/valid/type.rs

@@ -97,8 +97,8 @@ pub enum TypeError {
     UnsupportedSpecializedArrayLength(Handle<crate::Constant>),
     #[error("Array type {0:?} must have a length of one or more")]
     NonPositiveArrayLength(Handle<crate::Constant>),
-    #[error("Array stride {stride} is smaller than the base element size {base_size}")]
-    InsufficientArrayStride { stride: u32, base_size: u32 },
+    #[error("Array stride {stride} does not match the expected {expected}")]
+    InvalidArrayStride { stride: u32, expected: u32 },
     #[error("Field '{0}' can't be dynamically-sized, has type {1:?}")]
     InvalidDynamicArray(String, Handle<crate::Type>),
     #[error("Structure member[{index}] at {offset} overlaps the previous member")]
@@ -328,19 +328,20 @@ impl super::Validator {
                     return Err(TypeError::InvalidArrayBaseType(base));
                 }
 
-                //Note: `unwrap()` is fine, since `Layouter` goes first and calls it
-                let base_size = types[base].inner.size(constants);
-                if stride < base_size {
-                    return Err(TypeError::InsufficientArrayStride { stride, base_size });
+                let base_layout = self.layouter[base];
+                let expected_stride = base_layout.to_stride();
+                if stride != expected_stride {
+                    return Err(TypeError::InvalidArrayStride {
+                        stride,
+                        expected: expected_stride,
+                    });
                 }
 
-                let general_alignment = self.layouter[base].alignment;
+                let general_alignment = base_layout.alignment.get();
                 let uniform_layout = match base_info.uniform_layout {
                     Ok(base_alignment) => {
                         // combine the alignment requirements
-                        let align = ((base_alignment.unwrap().get() - 1)
-                            | (general_alignment.get() - 1))
-                            + 1;
+                        let align = base_alignment.unwrap().get().max(general_alignment);
                         if stride % align != 0 {
                             Err((
                                 handle,
@@ -357,9 +358,7 @@ impl super::Validator {
                 };
                 let storage_layout = match base_info.storage_layout {
                     Ok(base_alignment) => {
-                        let align = ((base_alignment.unwrap().get() - 1)
-                            | (general_alignment.get() - 1))
-                            + 1;
+                        let align = base_alignment.unwrap().get().max(general_alignment);
                         if stride % align != 0 {
                             Err((
                                 handle,

tests/in/access.wgsl

@@ -1,11 +1,15 @@
 // This snapshot tests accessing various containers, dereferencing pointers.
 
+struct AlignedWrapper {
+	@align(8) value: i32;
+};
+
 struct Bar {
 	matrix: mat4x4<f32>;
 	matrix_array: array<mat2x2<f32>, 2>;
 	atom: atomic<i32>;
-	arr: @stride(8) array<vec2<u32>, 2>;
-	data: @stride(8) array<i32>;
+	arr: array<vec2<u32>, 2>;
+	data: array<AlignedWrapper>;
 };
 
 @group(0) @binding(0)
@@ -27,17 +31,17 @@ fn foo(@builtin(vertex_index) vi: u32) -> @builtin(position) vec4<f32> {
 	let arr = bar.arr;
 	let index = 3u;
 	let b = bar.matrix[index].x;
-	let a = bar.data[arrayLength(&bar.data) - 2u];
+	let a = bar.data[arrayLength(&bar.data) - 2u].value;
 
 	// test pointer types
-	let data_pointer: ptr<storage, i32, read_write> = &bar.data[0];
+	let data_pointer: ptr<storage, i32, read_write> = &bar.data[0].value;
 	let foo_value = read_from_private(&foo);
 
 	// test storage stores
 	bar.matrix[1].z = 1.0;
 	bar.matrix = mat4x4<f32>(vec4<f32>(0.0), vec4<f32>(1.0), vec4<f32>(2.0), vec4<f32>(3.0));
 	bar.arr = array<vec2<u32>, 2>(vec2<u32>(0u), vec2<u32>(1u));
-	bar.data[1] = 1;
+	bar.data[1].value = 1;
 
 	// test array indexing
 	var c = array<i32, 5>(a, i32(b), 3, 4, 5);

tests/in/boids.wgsl

@@ -16,7 +16,7 @@ struct SimParams {
 };
 
 struct Particles {
-  particles : @stride(16) array<Particle>;
+  particles : array<Particle>;
 };
 
 @group(0) @binding(0) var<uniform> params : SimParams;

tests/in/collatz.wgsl

@@ -1,5 +1,5 @@
 struct PrimeIndices {
-    data: @stride(4) array<u32>;
+    data: array<u32>;
 }; // this is used as both input and output for convenience
 
 @group(0) @binding(0)

tests/in/shadow.wgsl

@@ -12,7 +12,7 @@ struct Light {
 };
 
 struct Lights {
-    data: @stride(96) array<Light>;
+    data: array<Light>;
 };
 
 @group(0) @binding(1)

tests/out/glsl/access.atomics.Compute.glsl

@@ -5,12 +5,15 @@ precision highp int;
 
 layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
 
+struct AlignedWrapper {
+    int value;
+};
 layout(std430) buffer Bar_block_0Compute {
     mat4x4 matrix;
     mat2x2 matrix_array[2];
     int atom;
     uvec2 arr[2];
-    int data[];
+    AlignedWrapper data[];
 } _group_0_binding_0_cs;
 
 

tests/out/glsl/access.foo.Vertex.glsl

@@ -3,12 +3,15 @@
 precision highp float;
 precision highp int;
 
+struct AlignedWrapper {
+    int value;
+};
 layout(std430) buffer Bar_block_0Vertex {
     mat4x4 matrix;
     mat2x2 matrix_array[2];
     int atom;
     uvec2 arr[2];
-    int data[];
+    AlignedWrapper data[];
 } _group_0_binding_0_vs;
 
 
@@ -26,12 +29,12 @@ void main() {
     mat4x4 matrix = _group_0_binding_0_vs.matrix;
     uvec2 arr[2] = _group_0_binding_0_vs.arr;
     float b = _group_0_binding_0_vs.matrix[3][0];
-    int a = _group_0_binding_0_vs.data[(uint(_group_0_binding_0_vs.data.length()) - 2u)];
-    float _e25 = read_from_private(foo_1);
+    int a = _group_0_binding_0_vs.data[(uint(_group_0_binding_0_vs.data.length()) - 2u)].value;
+    float _e27 = read_from_private(foo_1);
     _group_0_binding_0_vs.matrix[1][2] = 1.0;
     _group_0_binding_0_vs.matrix = mat4x4(vec4(0.0), vec4(1.0), vec4(2.0), vec4(3.0));
     _group_0_binding_0_vs.arr = uvec2[2](uvec2(0u), uvec2(1u));
-    _group_0_binding_0_vs.data[1] = 1;
+    _group_0_binding_0_vs.data[1].value = 1;
     c = int[5](a, int(b), 3, 4, 5);
     c[(vi + 1u)] = 42;
     int value = c[vi];