Convert binding and group attributes to expressions.
This CL updates the @group and @binding attributes to use
expressions instead of integers.
Bug: tint:1633
Change-Id: I91068874c104d5b84390f1617cb96265dda6e1e0
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/105801
Auto-Submit: Dan Sinclair <dsinclair@chromium.org>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc
index 841649d..7c06020 100644
--- a/src/tint/reader/wgsl/parser_impl.cc
+++ b/src/tint/reader/wgsl/parser_impl.cc
@@ -3441,15 +3441,16 @@
if (t == "binding") {
const char* use = "binding 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 binding expression");
+ }
match(Token::Type::kComma);
- return create<ast::BindingAttribute>(
- t.source(), create<ast::IntLiteralExpression>(
- val.value, ast::IntLiteralExpression::Suffix::kNone));
+ return create<ast::BindingAttribute>(t.source(), expr.value);
});
}
@@ -3478,15 +3479,16 @@
if (t == "group") {
const char* use = "group 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 group expression");
+ }
match(Token::Type::kComma);
- return create<ast::GroupAttribute>(
- t.source(), create<ast::IntLiteralExpression>(
- val.value, ast::IntLiteralExpression::Suffix::kNone));
+ return create<ast::GroupAttribute>(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 19434e6..0fc27dd 100644
--- a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
+++ b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
@@ -1017,10 +1017,10 @@
}
TEST_F(ParserImplErrorTest, GlobalDeclVarAttrBindingInvalidValue) {
- EXPECT("@binding(x) var i : i32;",
- R"(test.wgsl:1:10 error: expected signed integer literal for binding attribute
-@binding(x) var i : i32;
- ^
+ EXPECT("@binding(if) var i : i32;",
+ R"(test.wgsl:1:10 error: expected binding expression
+@binding(if) var i : i32;
+ ^^
)");
}
@@ -1041,10 +1041,10 @@
}
TEST_F(ParserImplErrorTest, GlobalDeclVarAttrBindingGroupValue) {
- EXPECT("@group(x) var i : i32;",
- R"(test.wgsl:1:8 error: expected signed integer literal for group attribute
-@group(x) var i : i32;
- ^
+ EXPECT("@group(if) var i : i32;",
+ R"(test.wgsl:1:8 error: expected group expression
+@group(if) var i : i32;
+ ^^
)");
}
diff --git a/src/tint/reader/wgsl/parser_impl_global_variable_decl_test.cc b/src/tint/reader/wgsl/parser_impl_global_variable_decl_test.cc
index 388ad67..5f051db 100644
--- a/src/tint/reader/wgsl/parser_impl_global_variable_decl_test.cc
+++ b/src/tint/reader/wgsl/parser_impl_global_variable_decl_test.cc
@@ -139,7 +139,7 @@
EXPECT_NE(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:10: expected signed integer literal for binding attribute");
+ EXPECT_EQ(p->error(), "1:10: expected binding expression");
}
TEST_F(ParserImplTest, GlobalVariableDecl_InvalidConstExpr) {
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 4c68f9b..86e5487 100644
--- a/src/tint/reader/wgsl/parser_impl_variable_attribute_test.cc
+++ b/src/tint/reader/wgsl/parser_impl_variable_attribute_test.cc
@@ -392,6 +392,31 @@
EXPECT_EQ(expr->suffix, ast::IntLiteralExpression::Suffix::kNone);
}
+TEST_F(ParserImplTest, Attribute_Binding_Expression) {
+ auto p = parser("binding(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::BindingAttribute>());
+
+ auto* binding = var_attr->As<ast::BindingAttribute>();
+ ASSERT_TRUE(binding->expr->Is<ast::BinaryExpression>());
+ auto* expr = binding->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_Binding_TrailingComma) {
auto p = parser("binding(4,)");
auto attr = p->attribute();
@@ -437,17 +462,17 @@
EXPECT_TRUE(attr.errored);
EXPECT_EQ(attr.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:9: expected signed integer literal for binding attribute");
+ EXPECT_EQ(p->error(), "1:9: expected binding expression");
}
TEST_F(ParserImplTest, Attribute_Binding_MissingInvalid) {
- auto p = parser("binding(nan)");
+ auto p = parser("binding(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:9: expected signed integer literal for binding attribute");
+ EXPECT_EQ(p->error(), "1:9: expected binding expression");
}
TEST_F(ParserImplTest, Attribute_group) {
@@ -468,6 +493,31 @@
EXPECT_EQ(expr->suffix, ast::IntLiteralExpression::Suffix::kNone);
}
+TEST_F(ParserImplTest, Attribute_group_expression) {
+ auto p = parser("group(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_FALSE(p->has_error());
+ ASSERT_NE(var_attr, nullptr);
+ ASSERT_TRUE(var_attr->Is<ast::GroupAttribute>());
+
+ auto* group = var_attr->As<ast::GroupAttribute>();
+ ASSERT_TRUE(group->expr->Is<ast::BinaryExpression>());
+ auto* expr = group->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_group_TrailingComma) {
auto p = parser("group(4,)");
auto attr = p->attribute();
@@ -513,17 +563,17 @@
EXPECT_TRUE(attr.errored);
EXPECT_EQ(attr.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:7: expected signed integer literal for group attribute");
+ EXPECT_EQ(p->error(), "1:7: expected group expression");
}
TEST_F(ParserImplTest, Attribute_Group_MissingInvalid) {
- auto p = parser("group(nan)");
+ auto p = parser("group(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:7: expected signed integer literal for group attribute");
+ EXPECT_EQ(p->error(), "1:7: expected group expression");
}
} // namespace
diff --git a/src/tint/resolver/attribute_validation_test.cc b/src/tint/resolver/attribute_validation_test.cc
index 317fab0..ce6baba 100644
--- a/src/tint/resolver/attribute_validation_test.cc
+++ b/src/tint/resolver/attribute_validation_test.cc
@@ -1562,6 +1562,83 @@
R"(12:34 error: interpolate attribute must only be used with @location)");
}
+using GroupAndBindingTest = ResolverTest;
+
+TEST_F(GroupAndBindingTest, Const_I32) {
+ GlobalConst("b", Expr(4_i));
+ GlobalConst("g", Expr(2_i));
+ GlobalVar("val", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()), Binding("b"),
+ Group("g"));
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(GroupAndBindingTest, Const_U32) {
+ GlobalConst("b", Expr(4_u));
+ GlobalConst("g", Expr(2_u));
+ GlobalVar("val", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()), Binding("b"),
+ Group("g"));
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(GroupAndBindingTest, Const_AInt) {
+ GlobalConst("b", Expr(4_a));
+ GlobalConst("g", Expr(2_a));
+ GlobalVar("val", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()), Binding("b"),
+ Group("g"));
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(GroupAndBindingTest, Binding_Negative) {
+ GlobalVar("val", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()),
+ Binding(Source{{12, 34}}, -2_i), Group(1_i));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: 'binding' value must be non-negative)");
+}
+
+TEST_F(GroupAndBindingTest, Binding_F32) {
+ GlobalVar("val", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()),
+ Binding(Source{{12, 34}}, 2.0_f), Group(1_u));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: 'binding' must be an i32 or u32 value)");
+}
+
+TEST_F(GroupAndBindingTest, Binding_AFloat) {
+ GlobalVar("val", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()),
+ Binding(Source{{12, 34}}, 2.0_a), Group(1_u));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: 'binding' must be an i32 or u32 value)");
+}
+
+TEST_F(GroupAndBindingTest, Group_Negative) {
+ GlobalVar("val", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()), Binding(2_u),
+ Group(Source{{12, 34}}, -1_i));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: 'group' value must be non-negative)");
+}
+
+TEST_F(GroupAndBindingTest, Group_F32) {
+ GlobalVar("val", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()), Binding(2_u),
+ Group(Source{{12, 34}}, 1.0_f));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: 'group' must be an i32 or u32 value)");
+}
+
+TEST_F(GroupAndBindingTest, Group_AFloat) {
+ GlobalVar("val", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()), Binding(2_u),
+ Group(Source{{12, 34}}, 1.0_a));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: 'group' must be an i32 or u32 value)");
+}
+
} // namespace
} // namespace InterpolateTests
diff --git a/src/tint/resolver/dependency_graph.cc b/src/tint/resolver/dependency_graph.cc
index 168734f..514e0cb 100644
--- a/src/tint/resolver/dependency_graph.cc
+++ b/src/tint/resolver/dependency_graph.cc
@@ -198,6 +198,7 @@
[&](const ast::Variable* var) {
Declare(var->symbol, var);
TraverseType(var->type);
+ TraverseAttributes(var->attributes);
if (var->constructor) {
TraverseExpression(var->constructor);
}
@@ -416,25 +417,38 @@
/// Traverses the attribute, performing symbol resolution and determining
/// global dependencies.
void TraverseAttribute(const ast::Attribute* attr) {
- if (auto* wg = attr->As<ast::WorkgroupAttribute>()) {
- TraverseExpression(wg->x);
- TraverseExpression(wg->y);
- TraverseExpression(wg->z);
- return;
- }
- if (auto* align = attr->As<ast::StructMemberAlignAttribute>()) {
- TraverseExpression(align->expr);
- return;
- }
- if (auto* size = attr->As<ast::StructMemberSizeAttribute>()) {
- TraverseExpression(size->expr);
+ bool handled = Switch(
+ attr,
+ [&](const ast::BindingAttribute* binding) {
+ TraverseExpression(binding->expr);
+ return true;
+ },
+ [&](const ast::GroupAttribute* group) {
+ TraverseExpression(group->expr);
+ return true;
+ },
+ [&](const ast::StructMemberAlignAttribute* align) {
+ TraverseExpression(align->expr);
+ return true;
+ },
+ [&](const ast::StructMemberSizeAttribute* size) {
+ TraverseExpression(size->expr);
+ return true;
+ },
+ [&](const ast::WorkgroupAttribute* wg) {
+ TraverseExpression(wg->x);
+ TraverseExpression(wg->y);
+ TraverseExpression(wg->z);
+ return true;
+ });
+ if (handled) {
return;
}
- if (attr->IsAnyOf<ast::BindingAttribute, ast::BuiltinAttribute, ast::GroupAttribute,
- ast::IdAttribute, ast::InternalAttribute, ast::InterpolateAttribute,
- ast::InvariantAttribute, ast::LocationAttribute, ast::StageAttribute,
- ast::StrideAttribute, ast::StructMemberOffsetAttribute>()) {
+ if (attr->IsAnyOf<ast::BuiltinAttribute, ast::IdAttribute, ast::InternalAttribute,
+ ast::InterpolateAttribute, ast::InvariantAttribute,
+ ast::LocationAttribute, 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 53bf3ee..150d65a 100644
--- a/src/tint/resolver/dependency_graph_test.cc
+++ b/src/tint/resolver/dependency_graph_test.cc
@@ -1289,6 +1289,9 @@
GlobalVar(Sym(), ty.storage_texture(ast::TextureDimension::k2d, ast::TexelFormat::kR32Float,
ast::Access::kRead)); //
GlobalVar(Sym(), ty.sampler(ast::SamplerKind::kSampler));
+
+ GlobalVar(Sym(), ty.i32(), utils::Vector{Binding(V), Group(V)});
+
Func(Sym(), utils::Empty, ty.void_(), utils::Empty);
#undef V
#undef T
diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc
index 0ae9cd9..eebd5ea 100644
--- a/src/tint/resolver/resolver.cc
+++ b/src/tint/resolver/resolver.cc
@@ -619,34 +619,50 @@
if (var->HasBindingPoint()) {
uint32_t binding = 0;
{
+ ExprEvalStageConstraint constraint{sem::EvaluationStage::kConstant, "@binding"};
+ TINT_SCOPED_ASSIGNMENT(expr_eval_stage_constraint_, constraint);
+
auto* attr = ast::GetAttribute<ast::BindingAttribute>(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 binding can be an expression.
+ if (!materialized->Type()->IsAnyOf<sem::I32, sem::U32>()) {
+ AddError("'binding' must be an i32 or u32 value", attr->source);
return nullptr;
}
- binding = c->As<uint32_t>();
+
+ auto const_value = materialized->ConstantValue();
+ auto value = const_value->As<AInt>();
+ if (value < 0) {
+ AddError("'binding' value must be non-negative", attr->source);
+ return nullptr;
+ }
+ binding = u32(value);
}
uint32_t group = 0;
{
+ ExprEvalStageConstraint constraint{sem::EvaluationStage::kConstant, "@group"};
+ TINT_SCOPED_ASSIGNMENT(expr_eval_stage_constraint_, constraint);
+
auto* attr = ast::GetAttribute<ast::GroupAttribute>(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 binding can be an expression.
+ if (!materialized->Type()->IsAnyOf<sem::I32, sem::U32>()) {
+ AddError("'group' must be an i32 or u32 value", attr->source);
return nullptr;
}
- group = c->As<uint32_t>();
+
+ auto const_value = materialized->ConstantValue();
+ auto value = const_value->As<AInt>();
+ if (value < 0) {
+ AddError("'group' value must be non-negative", attr->source);
+ return nullptr;
+ }
+ group = u32(value);
}
binding_point = {group, binding};
}
