commit dfa46d12ce63f131c437041d6d97a6d97c54c1b7
author James Price <jrprice@google.com> Tue Nov 12 23:05:45 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Nov 12 23:05:45 2024 +0000
tree f7e69c37db40bac3693af5e522e1afa6be287b3a
parent 295b81ecd1ac6b671eeed04efca88aa27a9e5dcc

[tint] Validate layout constraints for all address spaces

The WGSL spec has a non-normative note that the layout constraints
should be validated for all non-host-shareable address spaces, using
the same constraints as for storage.

See linked bug for details of why this is important.

Bug: 378725734
Change-Id: I3fb02506d8ded000dc3510bdc1b4a24a95089281
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/214754
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>

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

diff --git a/src/tint/lang/wgsl/resolver/address_space_layout_validation_test.cc b/src/tint/lang/wgsl/resolver/address_space_layout_validation_test.cc
index f1e14a3..6ef150c 100644
--- a/src/tint/lang/wgsl/resolver/address_space_layout_validation_test.cc
+++ b/src/tint/lang/wgsl/resolver/address_space_layout_validation_test.cc
@@ -730,7 +730,7 @@
     EXPECT_TRUE(r()->Resolve()) << r()->error();
 }
 
-TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall) {
+TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall_Storage) {
     // struct S {
     //   @align(4) vector : vec4u;
     //   scalar : u32;
@@ -754,5 +754,73 @@
 56:78 note: 'S' used in address space 'storage' here)");
 }
 
+TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall_Workgroup) {
+    // struct S {
+    //   @align(4) vector : vec4u;
+    //   scalar : u32;
+    // };
+    //
+    // var<workgroup> a : array<S, 4>;
+    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::kWorkgroup, Group(0_a));
+
+    ASSERT_FALSE(r()->Resolve());
+    EXPECT_EQ(
+        r()->error(),
+        R"(12:34 error: alignment must be a multiple of '16' bytes for the 'workgroup' address space
+56:78 note: 'S' used in address space 'workgroup' here)");
+}
+
+TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall_Private) {
+    // struct S {
+    //   @align(4) vector : vec4u;
+    //   scalar : u32;
+    // };
+    //
+    // var<private> a : array<S, 4>;
+    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::kPrivate, Group(0_a));
+
+    ASSERT_FALSE(r()->Resolve());
+    EXPECT_EQ(
+        r()->error(),
+        R"(12:34 error: alignment must be a multiple of '16' bytes for the 'private' address space
+56:78 note: 'S' used in address space 'private' here)");
+}
+
+TEST_F(ResolverAddressSpaceLayoutValidationTest, AlignAttributeTooSmall_Function) {
+    // struct S {
+    //   @align(4) vector : vec4u;
+    //   scalar : u32;
+    // };
+    //
+    // fn foo() {
+    //   var a : array<S, 4>;
+    // }
+    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::kFunction, Group(0_a));
+
+    ASSERT_FALSE(r()->Resolve());
+    EXPECT_EQ(
+        r()->error(),
+        R"(12:34 error: alignment must be a multiple of '16' bytes for the 'function' address space
+56:78 note: 'S' used in address space 'function' here)");
+}
+
 }  // namespace
 }  // namespace tint::resolver

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

diff --git a/src/tint/lang/wgsl/resolver/validator.cc b/src/tint/lang/wgsl/resolver/validator.cc
index 0c3a7a5..a9bcdf2 100644
--- a/src/tint/lang/wgsl/resolver/validator.cc
+++ b/src/tint/lang/wgsl/resolver/validator.cc
@@ -557,10 +557,6 @@
         return true;
     }
 
-    if (!core::IsHostShareable(address_space)) {
-        return true;
-    }
-
     auto note_usage = [&] {
         AddNote(source) << style::Type(store_ty->FriendlyName()) << " used in address space "
                         << style::Enum(address_space) << " here";