[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 <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
(cherry picked from commit ed15f8542825f25131c5a186e7de3737d49d327e)
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/212714
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Alan Baker <alanbaker@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
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 96b1340..f1e14a3 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
@@ -216,7 +216,7 @@
// 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 @@
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 @@
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 @@
// a : i32;
// b : i32;
// c : i32;
- // @align(1) @size(5) scalar : i32;
+ // @align(4) @size(5) scalar : i32;
// };
//
// struct Outer {
@@ -281,7 +281,7 @@
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 @@
/* 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 @@
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 @@
// 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 @@
// 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 @@
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 @@
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
diff --git a/src/tint/lang/wgsl/resolver/validator.cc b/src/tint/lang/wgsl/resolver/validator.cc
index dda482a..57cffb9 100644
--- a/src/tint/lang/wgsl/resolver/validator.cc
+++ b/src/tint/lang/wgsl/resolver/validator.cc
@@ -640,6 +640,22 @@
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;
+ }
+ }
}
}
diff --git a/webgpu-cts/compat-expectations.txt b/webgpu-cts/compat-expectations.txt
index b8c8466..e15c2b6 100644
--- a/webgpu-cts/compat-expectations.txt
+++ b/webgpu-cts/compat-expectations.txt
@@ -225,6 +225,10 @@
crbug.com/dawn/2086 [ intel-0x9bc5 ] webgpu:web_platform,canvas,configure:viewFormats:* [ Failure ]
crbug.com/dawn/2086 [ nvidia-0x2184 ] webgpu:web_platform,canvas,configure:viewFormats:* [ 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
diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt
index b3af340..91141ec 100644
--- a/webgpu-cts/expectations.txt
+++ b/webgpu-cts/expectations.txt
@@ -1655,6 +1655,10 @@
crbug.com/dawn/0000 [ nvidia-0x2184 ] webgpu:shader,execution,expression,call,builtin,fwidthFine:f32:vectorize="_undef_";non_uniform_discard=true [ Failure ]
crbug.com/dawn/0000 [ nvidia-0x2184 ] webgpu:shader,execution,expression,call,builtin,fwidthFine:f32:vectorize=4;non_uniform_discard=true [ 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 ]
+
################################################################################
# AMD Macbook 16" Qualification (NEEDS TRIAGE)
################################################################################
