encounter/dawn-cmake
1
0
Fork 0
mirror of https://github.com/encounter/dawn-cmake.git synced 2025-12-12 06:45:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

Deprecate the @stride attribute

Browse Source 
Update validation error for invalid uniform array element alignment.

Update tests to either remove the @stride attribute or use a different
element type.

Bug: tint:1381
Change-Id: I50b52cd78a34d9cd162fa5f2171a5fd35dcf3b79
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/77560
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
James Price
2022-01-20 22:11:07 +00:00
parent e04d0f40de
commit f6e5cc03bf
71 changed files with 575 additions and 512 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/reader/wgsl/parser_impl.cc
View File

@@ -3076,7 +3076,9 @@ Maybe<const ast::Decoration*> ParserImpl::decoration() {
auto val = expect_nonzero_positive_sint(use);
if (val.errored)
return Failure::kErrored;
deprecated(t.source(),
"the @stride attribute is deprecated; use a larger type if "
"necessary");
return create<ast::StrideDecoration>(t.source(), val.value);
});
}

26
src/reader/wgsl/parser_impl_error_msg_test.cc
View File

@@ -985,24 +985,24 @@ var i : array<u32, 3;
}
TEST_F(ParserImplErrorTest, GlobalDeclVarArrayDecoNotArray) {
EXPECT("var i : @stride(1) i32;",
EXPECT("var i : @location(1) i32;",
R"(test.wgsl:1:10 error: unexpected decorations
var i : @stride(1) i32;
^^^^^^
var i : @location(1) i32;
^^^^^^^^
)");
}
// TODO(crbug.com/tint/1382): Remove
TEST_F(ParserImplErrorTest, DEPRECATED_GlobalDeclVarArrayDecoMissingEnd) {
EXPECT(
"var i : [[stride(1) array<i32>;",
"var i : [[location(1) array<i32>;",
R"(test.wgsl:1:9 warning: use of deprecated language feature: [[decoration]] style decorations have been replaced with @decoration style
var i : [[stride(1) array<i32>;
var i : [[location(1) array<i32>;
^^
test.wgsl:1:21 error: expected ']]' for decoration list
var i : [[stride(1) array<i32>;
^^^^^
test.wgsl:1:23 error: expected ']]' for decoration list
var i : [[location(1) array<i32>;
^^^^^
)");
}
@@ -1025,14 +1025,14 @@ var i : [[stride 1)]] array<i32>;
TEST_F(ParserImplErrorTest,
DEPRECATED_GlobalDeclVarArrayDecoStrideMissingRParen) {
EXPECT(
"var i : [[stride(1]] array<i32>;",
"var i : [[location(1]] array<i32>;",
R"(test.wgsl:1:9 warning: use of deprecated language feature: [[decoration]] style decorations have been replaced with @decoration style
var i : [[stride(1]] array<i32>;
var i : [[location(1]] array<i32>;
^^
test.wgsl:1:19 error: expected ')' for stride decoration
var i : [[stride(1]] array<i32>;
^^
test.wgsl:1:21 error: expected ')' for location decoration
var i : [[location(1]] array<i32>;
^^
)");
}

13
src/reader/wgsl/parser_impl_type_decl_test.cc
View File

@@ -534,7 +534,10 @@ TEST_F(ParserImplTest, TypeDecl_Array_Decoration_MissingArray) {
EXPECT_FALSE(t.matched);
ASSERT_EQ(t.value, nullptr);
ASSERT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:2: unexpected decorations");
EXPECT_EQ(
p->error(),
R"(1:2: use of deprecated language feature: the @stride attribute is deprecated; use a larger type if necessary
1:2: unexpected decorations)");
}
TEST_F(ParserImplTest, TypeDecl_Array_Decoration_UnknownDecoration) {
@@ -546,6 +549,7 @@ TEST_F(ParserImplTest, TypeDecl_Array_Decoration_UnknownDecoration) {
ASSERT_TRUE(p->has_error());
EXPECT_EQ(p->error(), R"(1:2: expected decoration)");
}
TEST_F(ParserImplTest, TypeDecl_Array_Stride_MissingLeftParen) {
auto p = parser("@stride 4) array<f32, 5>");
auto t = p->type_decl();
@@ -563,7 +567,10 @@ TEST_F(ParserImplTest, TypeDecl_Array_Stride_MissingRightParen) {
EXPECT_FALSE(t.matched);
ASSERT_EQ(t.value, nullptr);
ASSERT_TRUE(p->has_error());
EXPECT_EQ(p->error(), R"(1:11: expected ')' for stride decoration)");
EXPECT_EQ(
p->error(),
R"(1:2: use of deprecated language feature: the @stride attribute is deprecated; use a larger type if necessary
1:11: expected ')' for stride decoration)");
}
TEST_F(ParserImplTest, TypeDecl_Array_Stride_MissingValue) {
@@ -610,6 +617,7 @@ TEST_F(ParserImplTest,
EXPECT_EQ(
p->error(),
R"(1:1: use of deprecated language feature: [[decoration]] style decorations have been replaced with @decoration style
1:3: use of deprecated language feature: the @stride attribute is deprecated; use a larger type if necessary
1:14: expected ']]' for decoration list)");
}
@@ -638,6 +646,7 @@ TEST_F(ParserImplTest, DEPRECATED_TypeDecl_Array_Stride_MissingRightParen) {
EXPECT_EQ(
p->error(),
R"(1:1: use of deprecated language feature: [[decoration]] style decorations have been replaced with @decoration style
1:3: use of deprecated language feature: the @stride attribute is deprecated; use a larger type if necessary
1:11: expected ')' for stride decoration)");
}

14
src/reader/wgsl/parser_impl_variable_ident_decl_test.cc
View File

@@ -77,7 +77,7 @@ TEST_F(ParserImplTest, VariableIdentDecl_InvalidIdent) {
}
TEST_F(ParserImplTest, VariableIdentDecl_NonAccessDecoFail) {
auto p = parser("my_var : @stride(1) S");
auto p = parser("my_var : @location(1) S");
auto* mem = Member("a", ty.i32(), ast::DecorationList{});
ast::StructMemberList members;
@@ -94,11 +94,11 @@ TEST_F(ParserImplTest, VariableIdentDecl_NonAccessDecoFail) {
}
TEST_F(ParserImplTest, VariableIdentDecl_DecorationMissingRightParen) {
auto p = parser("my_var : @stride(4 S");
auto p = parser("my_var : @location(4 S");
auto decl = p->expect_variable_ident_decl("test");
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(decl.errored);
ASSERT_EQ(p->error(), "1:20: expected ')' for stride decoration");
ASSERT_EQ(p->error(), "1:22: expected ')' for location decoration");
}
TEST_F(ParserImplTest, VariableIdentDecl_DecorationMissingLeftParen) {
@@ -112,27 +112,27 @@ TEST_F(ParserImplTest, VariableIdentDecl_DecorationMissingLeftParen) {
// TODO(crbug.com/tint/1382): Remove
TEST_F(ParserImplTest,
DEPRECATED_VariableIdentDecl_DecorationMissingRightBlock) {
auto p = parser("my_var : [[stride(4) S");
auto p = parser("my_var : [[location(4) S");
auto decl = p->expect_variable_ident_decl("test");
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(decl.errored);
ASSERT_EQ(
p->error(),
R"(1:10: use of deprecated language feature: [[decoration]] style decorations have been replaced with @decoration style
1:22: expected ']]' for decoration list)");
1:24: expected ']]' for decoration list)");
}
// TODO(crbug.com/tint/1382): Remove
TEST_F(ParserImplTest,
DEPRECATED_VariableIdentDecl_DecorationMissingRightParen) {
auto p = parser("my_var : [[stride(4]] S");
auto p = parser("my_var : [[location(4]] S");
auto decl = p->expect_variable_ident_decl("test");
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(decl.errored);
ASSERT_EQ(
p->error(),
R"(1:10: use of deprecated language feature: [[decoration]] style decorations have been replaced with @decoration style
1:20: expected ')' for stride decoration)");
1:22: expected ')' for location decoration)");
}
// TODO(crbug.com/tint/1382): Remove

26
src/resolver/resolver_validation.cc
View File

@@ -321,15 +321,29 @@ bool Resolver::ValidateStorageClassLayout(const sem::Struct* str,
// bytes above, so we only need to validate that stride is a multiple of
// 16 bytes.
if (arr->Stride() % 16 != 0) {
// Since WGSL has no stride attribute, try to provide a useful hint
// for how the shader author can resolve the issue.
std::string hint;
if (arr->ElemType()->is_scalar()) {
hint =
"Consider using a vector or struct as the element type "
"instead.";
} else if (auto* vec = arr->ElemType()->As<sem::Vector>();
vec && vec->type()->Size() == 4) {
hint = "Consider using a vec4 instead.";
} else if (arr->ElemType()->Is<sem::Struct>()) {
hint =
"Consider using the @size attribute on the last struct member.";
} else {
hint =
"Consider wrapping the element type in a struct and using the "
"@size attribute.";
}
AddError(
"uniform storage requires that array elements be aligned to 16 "
"bytes, but array stride of '" +
"bytes, but array element alignment of '" +
member_name_of(m) + "' is currently " +
std::to_string(arr->Stride()) +
". Consider setting @stride(" +
std::to_string(
utils::RoundUp(required_align, arr->Stride())) +
") on the array type",
std::to_string(arr->Stride()) + ". " + hint,
m->Declaration()->type->source);
AddNote("see layout of struct:\n" + str->Layout(builder_->Symbols()),
str->Declaration()->source);

93
src/resolver/storage_class_layout_validation_test.cc
View File

@@ -378,8 +378,8 @@ TEST_F(ResolverStorageClassLayoutValidationTest,
// Detect array stride must be a multiple of 16 bytes for uniform buffers
TEST_F(ResolverStorageClassLayoutValidationTest,
UniformBuffer_InvalidArrayStride) {
// type Inner = @stride(8) array<f32, 10>;
UniformBuffer_InvalidArrayStride_Scalar) {
// type Inner = array<f32, 10>;
//
// [[block]]
// struct Outer {
@@ -390,7 +390,7 @@ TEST_F(ResolverStorageClassLayoutValidationTest,
// @group(0) @binding(0)
// var<uniform> a : Outer;
Alias("Inner", ty.array(ty.f32(), 10, 8));
Alias("Inner", ty.array(ty.f32(), 10));
Structure(Source{{12, 34}}, "Outer",
{
@@ -405,10 +405,93 @@ TEST_F(ResolverStorageClassLayoutValidationTest,
ASSERT_FALSE(r()->Resolve());
EXPECT_EQ(
r()->error(),
R"(34:56 error: uniform storage requires that array elements be aligned to 16 bytes, but array stride of 'inner' is currently 8. Consider setting @stride(16) on the array type
R"(34:56 error: uniform storage requires that array elements be aligned to 16 bytes, but array element alignment of 'inner' is currently 4. Consider using a vector or struct as the element type instead.
12:34 note: see layout of struct:
/* align(4) size(44) */ struct Outer {
/* offset( 0) align(4) size(40) */ inner : array<f32, 10>;
/* offset(40) align(4) size( 4) */ scalar : i32;
/* */ };
78:90 note: see declaration of variable)");
}
TEST_F(ResolverStorageClassLayoutValidationTest,
UniformBuffer_InvalidArrayStride_Vector) {
// type Inner = array<vec2<f32>, 10>;
//
// [[block]]
// struct Outer {
// inner : Inner;
// scalar : i32;
// };
//
// @group(0) @binding(0)
// var<uniform> a : Outer;
Alias("Inner", ty.array(ty.vec2<f32>(), 10));
Structure(Source{{12, 34}}, "Outer",
{
Member("inner", ty.type_name(Source{{34, 56}}, "Inner")),
Member("scalar", ty.i32()),
},
{StructBlock()});
Global(Source{{78, 90}}, "a", ty.type_name("Outer"),
ast::StorageClass::kUniform, GroupAndBinding(0, 0));
ASSERT_FALSE(r()->Resolve());
EXPECT_EQ(
r()->error(),
R"(34:56 error: uniform storage requires that array elements be aligned to 16 bytes, but array element alignment of 'inner' is currently 8. Consider using a vec4 instead.
12:34 note: see layout of struct:
/* align(8) size(88) */ struct Outer {
/* offset( 0) align(8) size(80) */ inner : array<vec2<f32>, 10>;
/* offset(80) align(4) size( 4) */ scalar : i32;
/* offset(84) align(1) size( 4) */ // -- implicit struct size padding --;
/* */ };
78:90 note: see declaration of variable)");
}
TEST_F(ResolverStorageClassLayoutValidationTest,
UniformBuffer_InvalidArrayStride_Struct) {
// struct ArrayElem {
// a : f32;
// b : i32;
// }
// type Inner = array<ArrayElem, 10>;
//
// [[block]]
// struct Outer {
// inner : Inner;
// scalar : i32;
// };
//
// @group(0) @binding(0)
// var<uniform> a : Outer;
auto* array_elem = Structure("ArrayElem", {
Member("a", ty.f32()),
Member("b", ty.i32()),
});
Alias("Inner", ty.array(ty.Of(array_elem), 10));
Structure(Source{{12, 34}}, "Outer",
{
Member("inner", ty.type_name(Source{{34, 56}}, "Inner")),
Member("scalar", ty.i32()),
},
{StructBlock()});
Global(Source{{78, 90}}, "a", ty.type_name("Outer"),
ast::StorageClass::kUniform, GroupAndBinding(0, 0));
ASSERT_FALSE(r()->Resolve());
EXPECT_EQ(
r()->error(),
R"(34:56 error: uniform storage requires that array elements be aligned to 16 bytes, but array element alignment of 'inner' is currently 8. Consider using the @size attribute on the last struct member.
12:34 note: see layout of struct:
/* align(4) size(84) */ struct Outer {
/* offset( 0) align(4) size(80) */ inner : @stride(8) array<f32, 10>;
/* offset( 0) align(4) size(80) */ inner : array<ArrayElem, 10>;
/* offset(80) align(4) size( 4) */ scalar : i32;
/* */ };
78:90 note: see declaration of variable)");