[tint] Validate that @align() is large enough

Make sure that `n = k × RequiredAlignOf(T,C)` as per the spec, when
`@align(n)` is applied to the member of a structure that is used in a
host-shareable address space.

Suppress some CTS tests until they are updated upstream.

Fixed: 375123371
Include-Ci-Only-Tests: true
Change-Id: I3240b9ab0a42986e918a1c6a86268844861b9fed
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/212315
Commit-Queue: James Price <[email protected]>
Reviewed-by: dan sinclair <[email protected]>

Author: jrprice

src/tint/lang/wgsl/resolver/address_space_layout_validation_test.cc

@@ -216,7 +216,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_UnalignedMember_A
 // multiple of 16 bytes
 TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_MembersOffsetNotMultipleOf16) {
     // struct Inner {
-    //   @align(1) @size(5) scalar : i32;
+    //   @align(4) @size(5) scalar : i32;
     // };
     //
     // struct Outer {
@@ -229,7 +229,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_MembersOffsetNotM
 
     Structure(Ident(Source{{12, 34}}, "Inner"),
               Vector{
-                  Member("scalar", ty.i32(), Vector{MemberAlign(1_i), MemberSize(5_a)}),
+                  Member("scalar", ty.i32(), Vector{MemberAlign(4_i), MemberSize(5_a)}),
               });
 
     Structure(Source{{34, 56}}, "Outer",
@@ -247,13 +247,13 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, UniformBuffer_MembersOffsetNotM
         R"(78:90 error: 'uniform' storage requires that the number of bytes between the start of the previous member of type struct and the current member be a multiple of 16 bytes, but there are currently 8 bytes between 'inner' and 'scalar'. Consider setting '@align(16)' on this member
 note: see layout of struct:
 /*            align(4) size(12) */ struct Outer {
-/* offset( 0) align(1) size( 5) */   inner : Inner,
-/* offset( 5) align(1) size( 3) */   // -- implicit field alignment padding --
+/* offset( 0) align(4) size( 8) */   inner : Inner,
 /* offset( 8) align(4) size( 4) */   scalar : i32,
 /*                              */ };
 12:34 note: and layout of previous member struct:
-/*           align(1) size(5) */ struct Inner {
-/* offset(0) align(1) size(5) */   scalar : i32,
+/*           align(4) size(8) */ struct Inner {
+/* offset(0) align(4) size(5) */   scalar : i32,
+/* offset(5) align(1) size(3) */   // -- implicit struct size padding --
 /*                            */ };
 22:24 note: 'Outer' used in address space 'uniform' here)");
 }
@@ -265,7 +265,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest,
     //   a : i32;
     //   b : i32;
     //   c : i32;
-    //   @align(1) @size(5) scalar : i32;
+    //   @align(4) @size(5) scalar : i32;
     // };
     //
     // struct Outer {
@@ -281,7 +281,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest,
                   Member("a", ty.i32()),
                   Member("b", ty.i32()),
                   Member("c", ty.i32()),
-                  Member("scalar", ty.i32(), Vector{MemberAlign(1_i), MemberSize(5_a)}),
+                  Member("scalar", ty.i32(), Vector{MemberAlign(4_i), MemberSize(5_a)}),
               });
 
     Structure(Source{{34, 56}}, "Outer",
@@ -307,7 +307,7 @@ note: see layout of struct:
 /* offset( 0) align(4) size( 4) */   a : i32,
 /* offset( 4) align(4) size( 4) */   b : i32,
 /* offset( 8) align(4) size( 4) */   c : i32,
-/* offset(12) align(1) size( 5) */   scalar : i32,
+/* offset(12) align(4) size( 5) */   scalar : i32,
 /* offset(17) align(1) size( 3) */   // -- implicit struct size padding --
 /*                              */ };
 22:24 note: 'Outer' used in address space 'uniform' here)");
@@ -316,7 +316,7 @@ note: see layout of struct:
 TEST_F(ResolverAddressSpaceLayoutValidationTest,
        UniformBuffer_MembersOffsetNotMultipleOf16_SuggestedFix) {
     // struct Inner {
-    //   @align(1) @size(5) scalar : i32;
+    //   @align(4) @size(5) scalar : i32;
     // };
     //
     // struct Outer {
@@ -328,7 +328,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest,
     // var<uniform> a : Outer;
 
     Structure("Inner", Vector{
-                           Member("scalar", ty.i32(), Vector{MemberAlign(1_i), MemberSize(5_a)}),
+                           Member("scalar", ty.i32(), Vector{MemberAlign(4_i), MemberSize(5_a)}),
                        });
 
     Structure("Outer", Vector{
@@ -659,7 +659,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, RelaxedUniformLayout_MemberOffs
     // enable chromium_internal_relaxed_uniform_layout;
     //
     // struct Inner {
-    //   @align(1) @size(5) scalar : i32;
+    //   @align(4) @size(5) scalar : i32;
     // };
     //
     // struct Outer {
@@ -673,7 +673,7 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, RelaxedUniformLayout_MemberOffs
     Enable(wgsl::Extension::kChromiumInternalRelaxedUniformLayout);
 
     Structure("Inner", Vector{
-                           Member("scalar", ty.i32(), Vector{MemberAlign(1_i), MemberSize(5_a)}),
+                           Member("scalar", ty.i32(), Vector{MemberAlign(4_i), MemberSize(5_a)}),
                        });
 
     Structure("Outer", Vector{
@@ -730,5 +730,29 @@ TEST_F(ResolverAddressSpaceLayoutValidationTest, RelaxedUniformLayout_ArrayStrid
     EXPECT_TRUE(r()->Resolve()) << r()->error();
 }
 
+TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall) {
+    // struct S {
+    //   @align(4) vector : vec4u;
+    //   scalar : u32;
+    // };
+    //
+    // @group(0) @binding(0)
+    // var<storage, read_write> a : array<S>;
+    Structure(
+        "S", Vector{
+                 Member("vector", ty.vec4<u32>(), Vector{MemberAlign(Expr(Source{{12, 34}}, 4_a))}),
+                 Member("scalar", ty.u32()),
+             });
+
+    GlobalVar(Source{{56, 78}}, "a", ty("S"), core::AddressSpace::kStorage,
+              core::Access::kReadWrite, Group(0_a), Binding(0_a));
+
+    ASSERT_FALSE(r()->Resolve());
+    EXPECT_EQ(
+        r()->error(),
+        R"(12:34 error: alignment must be a multiple of '16' bytes for the 'storage' address space
+56:78 note: 'S' used in address space 'storage' here)");
+}
+
 }  // namespace
 }  // namespace tint::resolver

src/tint/lang/wgsl/resolver/validator.cc

@@ -641,6 +641,22 @@ bool Validator::AddressSpaceLayout(const core::type::Type* store_ty,
                     return false;
                 }
             }
+
+            // If an alignment was explicitly specified, we need to validate that it satisfies the
+            // alignment requirement of the address space.
+            auto* align_attr =
+                ast::GetAttribute<ast::StructMemberAlignAttribute>(m->Declaration()->attributes);
+            if (align_attr && !enabled_extensions_.Contains(
+                                  wgsl::Extension::kChromiumInternalRelaxedUniformLayout)) {
+                auto align = sem_.GetVal(align_attr->expr)->ConstantValue()->ValueAs<uint32_t>();
+                if (align % required_align != 0) {
+                    AddError(align_attr->expr->source)
+                        << "alignment must be a multiple of " << style::Literal(required_align)
+                        << " bytes for the " << style::Enum(address_space) << " address space";
+                    note_usage();
+                    return false;
+                }
+            }
         }
     }
 

webgpu-cts/compat-expectations.txt

@@ -273,6 +273,10 @@ crbug.com/dawn/372654300 webgpu:api,validation,render_pipeline,resource_compatib
 crbug.com/dawn/372654300 webgpu:api,validation,render_pipeline,resource_compatibility:resource_compatibility:stage="vertex";apiResource="texture_uint_cube-array_false" [ Failure ]
 crbug.com/dawn/372654300 webgpu:api,validation,render_pipeline,resource_compatibility:resource_compatibility:stage="vertex";apiResource="texture_unfilterable-float_cube-array_false" [ Failure ]
 
+# Failures due to change in `@align()` validation.
+crbug.com/375467276 webgpu:shader,execution,expression,access,structure,index:buffer_align:* [ Failure ]
+crbug.com/375467276 webgpu:shader,validation,shader_io,align:* [ Failure ]
+
 ### This section represents things that will require Compat validation
 ### These tests will never pass, but should be skipped in CTS once Compat
 ### validation has been added

webgpu-cts/expectations.txt

@@ -1457,6 +1457,10 @@ crbug.com/dawn/0000 [ nvidia-0x2184 nvidia_lt_535.183.01 ] webgpu:shader,executi
 crbug.com/dawn/366000875 webgpu:shader,validation,decl,var:var_access_mode_bad_other_template_contents:accessMode="read";prefix="storage,";suffix="," [ Failure ]
 crbug.com/dawn/366000875 webgpu:shader,validation,decl,var:var_access_mode_bad_other_template_contents:accessMode="read_write";prefix="storage,";suffix="," [ Failure ]
 
+# Failures due to change in `@align()` validation.
+crbug.com/375467276 webgpu:shader,execution,expression,access,structure,index:buffer_align:* [ Failure ]
+crbug.com/375467276 webgpu:shader,validation,shader_io,align:* [ Failure ]
+
 # Minor accuracy issue with the introduction of texture sample bias clampling
 # specific subtest case: samplePoints="spiral";addressModeU="clamp-to-edge";addressModeV="mirror-repeat";minFilter="linear"
 crbug.com/dawn/371033198 [ amd mac ] webgpu:shader,execution,expression,call,builtin,textureSampleBias:sampled_2d_coords:format="bc1-rgba-unorm";offset=true [ Failure ]