Convert the location attribute to expressions.
This CL updates the @location attribute to use expressions instead of
integers.
Bug: tint:1633
Change-Id: If4dfca6d39e5134bb173209414ad8d2528c8095d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/106121
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc
index f15fcce..8d6040d 100644
--- a/src/tint/reader/wgsl/parser_impl.cc
+++ b/src/tint/reader/wgsl/parser_impl.cc
@@ -3539,15 +3539,16 @@
if (t == "location") {
const char* use = "location attribute";
return expect_paren_block(use, [&]() -> Result {
- auto val = expect_positive_sint(use);
- if (val.errored) {
+ auto expr = expression();
+ if (expr.errored) {
return Failure::kErrored;
}
+ if (!expr.matched) {
+ return add_error(peek(), "expected location expression");
+ }
match(Token::Type::kComma);
- return builder_.Location(t.source(),
- create<ast::IntLiteralExpression>(
- val.value, ast::IntLiteralExpression::Suffix::kNone));
+ return builder_.Location(t.source(), expr.value);
});
}
diff --git a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
index a29c29c..9416a4d 100644
--- a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
+++ b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
@@ -959,10 +959,10 @@
}
TEST_F(ParserImplErrorTest, GlobalDeclVarAttrLocationInvalidValue) {
- EXPECT("@location(x) var i : i32;",
- R"(test.wgsl:1:11 error: expected signed integer literal for location attribute
-@location(x) var i : i32;
- ^
+ EXPECT("@location(if) var i : i32;",
+ R"(test.wgsl:1:11 error: expected location expression
+@location(if) var i : i32;
+ ^^
)");
}
diff --git a/src/tint/reader/wgsl/parser_impl_struct_attribute_decl_test.cc b/src/tint/reader/wgsl/parser_impl_struct_attribute_decl_test.cc
index e015033..051104b 100644
--- a/src/tint/reader/wgsl/parser_impl_struct_attribute_decl_test.cc
+++ b/src/tint/reader/wgsl/parser_impl_struct_attribute_decl_test.cc
@@ -46,7 +46,7 @@
EXPECT_TRUE(attrs.errored);
EXPECT_FALSE(attrs.matched);
EXPECT_TRUE(attrs.value.IsEmpty());
- EXPECT_EQ(p->error(), "1:11: expected signed integer literal for location attribute");
+ EXPECT_EQ(p->error(), "1:11: expected location expression");
}
TEST_F(ParserImplTest, AttributeDecl_MissingParenRight) {
diff --git a/src/tint/reader/wgsl/parser_impl_variable_attribute_test.cc b/src/tint/reader/wgsl/parser_impl_variable_attribute_test.cc
index 4f3f1b9..1b538a2 100644
--- a/src/tint/reader/wgsl/parser_impl_variable_attribute_test.cc
+++ b/src/tint/reader/wgsl/parser_impl_variable_attribute_test.cc
@@ -133,6 +133,31 @@
EXPECT_EQ(exp->value, 4u);
}
+TEST_F(ParserImplTest, Attribute_Location_Expression) {
+ auto p = parser("location(4 + 5)");
+ auto attr = p->attribute();
+ EXPECT_TRUE(attr.matched);
+ EXPECT_FALSE(attr.errored);
+ ASSERT_NE(attr.value, nullptr);
+ auto* var_attr = attr.value->As<ast::Attribute>();
+ ASSERT_NE(var_attr, nullptr);
+ ASSERT_FALSE(p->has_error());
+ ASSERT_TRUE(var_attr->Is<ast::LocationAttribute>());
+
+ auto* loc = var_attr->As<ast::LocationAttribute>();
+ ASSERT_TRUE(loc->expr->Is<ast::BinaryExpression>());
+ auto* expr = loc->expr->As<ast::BinaryExpression>();
+
+ EXPECT_EQ(ast::BinaryOp::kAdd, expr->op);
+ auto* v = expr->lhs->As<ast::IntLiteralExpression>();
+ ASSERT_NE(nullptr, v);
+ EXPECT_EQ(v->value, 4u);
+
+ v = expr->rhs->As<ast::IntLiteralExpression>();
+ ASSERT_NE(nullptr, v);
+ EXPECT_EQ(v->value, 5u);
+}
+
TEST_F(ParserImplTest, Attribute_Location_TrailingComma) {
auto p = parser("location(4,)");
auto attr = p->attribute();
@@ -177,17 +202,17 @@
EXPECT_TRUE(attr.errored);
EXPECT_EQ(attr.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:10: expected signed integer literal for location attribute");
+ EXPECT_EQ(p->error(), "1:10: expected location expression");
}
TEST_F(ParserImplTest, Attribute_Location_MissingInvalid) {
- auto p = parser("location(nan)");
+ auto p = parser("location(if)");
auto attr = p->attribute();
EXPECT_FALSE(attr.matched);
EXPECT_TRUE(attr.errored);
EXPECT_EQ(attr.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:10: expected signed integer literal for location attribute");
+ EXPECT_EQ(p->error(), "1:10: expected location expression");
}
struct BuiltinData {
diff --git a/src/tint/resolver/dependency_graph.cc b/src/tint/resolver/dependency_graph.cc
index 4d67023..e84eec5 100644
--- a/src/tint/resolver/dependency_graph.cc
+++ b/src/tint/resolver/dependency_graph.cc
@@ -431,6 +431,10 @@
TraverseExpression(id->expr);
return true;
},
+ [&](const ast::LocationAttribute* loc) {
+ TraverseExpression(loc->expr);
+ return true;
+ },
[&](const ast::StructMemberAlignAttribute* align) {
TraverseExpression(align->expr);
return true;
@@ -450,8 +454,8 @@
}
if (attr->IsAnyOf<ast::BuiltinAttribute, ast::InternalAttribute, ast::InterpolateAttribute,
- ast::InvariantAttribute, ast::LocationAttribute, ast::StageAttribute,
- ast::StrideAttribute, ast::StructMemberOffsetAttribute>()) {
+ ast::InvariantAttribute, ast::StageAttribute, ast::StrideAttribute,
+ ast::StructMemberOffsetAttribute>()) {
return;
}
diff --git a/src/tint/resolver/dependency_graph_test.cc b/src/tint/resolver/dependency_graph_test.cc
index 138507a..724a527 100644
--- a/src/tint/resolver/dependency_graph_test.cc
+++ b/src/tint/resolver/dependency_graph_test.cc
@@ -1291,6 +1291,7 @@
GlobalVar(Sym(), ty.sampler(ast::SamplerKind::kSampler));
GlobalVar(Sym(), ty.i32(), utils::Vector{Binding(V), Group(V)});
+ GlobalVar(Sym(), ty.i32(), utils::Vector{Location(V)});
Override(Sym(), ty.i32(), utils::Vector{Id(V)});
Func(Sym(), utils::Empty, ty.void_(), utils::Empty);
diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc
index 56f9246..c252473 100644
--- a/src/tint/resolver/resolver.cc
+++ b/src/tint/resolver/resolver.cc
@@ -675,17 +675,22 @@
std::optional<uint32_t> location;
if (auto* attr = ast::GetAttribute<ast::LocationAttribute>(var->attributes)) {
- auto* materialize = Materialize(Expression(attr->expr));
- if (!materialize) {
+ auto* materialized = Materialize(Expression(attr->expr));
+ if (!materialized) {
return nullptr;
}
- auto* c = materialize->ConstantValue();
- if (!c) {
- // TODO(crbug.com/tint/1633): Add error message about invalid materialization
- // when location can be an expression.
+ if (!materialized->Type()->IsAnyOf<sem::I32, sem::U32>()) {
+ AddError("'location' must be an i32 or u32 value", attr->source);
return nullptr;
}
- location = c->As<uint32_t>();
+
+ auto const_value = materialized->ConstantValue();
+ auto value = const_value->As<AInt>();
+ if (value < 0) {
+ AddError("'location' value must be non-negative", attr->source);
+ return nullptr;
+ }
+ location = u32(value);
}
sem = builder_->create<sem::GlobalVariable>(
