tint: Fix uniformity ICE during error reporting wrt non-uniform pointer parameters
Added support for reporting when pointer parameters point to non-uniform
values. Also add support for binary expressions results that may be
non-uniform.
Bug: chromium:1374534
Change-Id: Ia51557e3a984c69a39f2878c964bf07085599809
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/106560
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
diff --git a/src/tint/resolver/uniformity.cc b/src/tint/resolver/uniformity.cc
index d72e156..3b089c3 100644
--- a/src/tint/resolver/uniformity.cc
+++ b/src/tint/resolver/uniformity.cc
@@ -831,7 +831,7 @@
// Create input nodes for any variables declared before this loop.
for (auto* v : current_function_->local_var_decls) {
auto name = builder_->Symbols().NameFor(v->Declaration()->symbol);
- auto* in_node = CreateNode(name + "_value_loop_in");
+ auto* in_node = CreateNode(name + "_value_loop_in", v->Declaration());
in_node->AddEdge(current_function_->variables.Get(v));
info.var_in_nodes[v] = in_node;
current_function_->variables.Set(v, in_node);
@@ -1111,7 +1111,7 @@
} else {
auto [cf1, v1] = ProcessExpression(cf, b->lhs);
auto [cf2, v2] = ProcessExpression(cf1, b->rhs);
- auto* result = CreateNode("binary_expr_result");
+ auto* result = CreateNode("binary_expr_result", b);
result->AddEdge(v1);
result->AddEdge(v2);
return std::pair<Node*, Node*>(cf2, result);
@@ -1527,40 +1527,49 @@
// the actual cause of divergence.
}
+ auto get_var_type = [&](const sem::Variable* var) {
+ switch (var->AddressSpace()) {
+ case ast::AddressSpace::kStorage:
+ return "read_write storage buffer ";
+ case ast::AddressSpace::kWorkgroup:
+ return "workgroup storage variable ";
+ case ast::AddressSpace::kPrivate:
+ return "module-scope private variable ";
+ default:
+ if (ast::HasAttribute<ast::BuiltinAttribute>(var->Declaration()->attributes)) {
+ return "builtin ";
+ } else if (ast::HasAttribute<ast::LocationAttribute>(
+ var->Declaration()->attributes)) {
+ return "user-defined input ";
+ } else {
+ // TODO(jrprice): Provide more info for this case.
+ }
+ break;
+ }
+ return "";
+ };
+
// Show the source of the non-uniform value.
Switch(
non_uniform_source->ast,
[&](const ast::IdentifierExpression* ident) {
- std::string var_type = "";
auto* var = sem_.Get<sem::VariableUser>(ident)->Variable();
- switch (var->AddressSpace()) {
- case ast::AddressSpace::kStorage:
- var_type = "read_write storage buffer ";
- break;
- case ast::AddressSpace::kWorkgroup:
- var_type = "workgroup storage variable ";
- break;
- case ast::AddressSpace::kPrivate:
- var_type = "module-scope private variable ";
- break;
- default:
- if (ast::HasAttribute<ast::BuiltinAttribute>(
- var->Declaration()->attributes)) {
- var_type = "builtin ";
- } else if (ast::HasAttribute<ast::LocationAttribute>(
- var->Declaration()->attributes)) {
- var_type = "user-defined input ";
- } else {
- // TODO(jrprice): Provide more info for this case.
- }
- break;
- }
+ std::string var_type = get_var_type(var);
diagnostics_.add_note(diag::System::Resolver,
"reading from " + var_type + "'" +
builder_->Symbols().NameFor(ident->symbol) +
"' may result in a non-uniform value",
ident->source);
},
+ [&](const ast::Variable* v) {
+ auto* var = sem_.Get(v);
+ std::string var_type = get_var_type(var);
+ diagnostics_.add_note(diag::System::Resolver,
+ "reading from " + var_type + "'" +
+ builder_->Symbols().NameFor(v->symbol) +
+ "' may result in a non-uniform value",
+ v->source);
+ },
[&](const ast::CallExpression* c) {
auto target_name = builder_->Symbols().NameFor(
c->target.name->As<ast::IdentifierExpression>()->symbol);
@@ -1600,6 +1609,10 @@
}
}
},
+ [&](const ast::Expression* e) {
+ diagnostics_.add_note(diag::System::Resolver,
+ "result of expression may be non-uniform", e->source);
+ },
[&](Default) {
TINT_ICE(Resolver, diagnostics_) << "unhandled source of non-uniformity";
});
diff --git a/src/tint/resolver/uniformity_test.cc b/src/tint/resolver/uniformity_test.cc
index 79047d1..6ba3410 100644
--- a/src/tint/resolver/uniformity_test.cc
+++ b/src/tint/resolver/uniformity_test.cc
@@ -7962,6 +7962,99 @@
)");
}
+TEST_F(UniformityAnalysisTest,
+ Error_ParameterRequiredToBeUniformForSubsequentControlFlow_ViaPointer) {
+ // Make sure we correctly identify the function call as the source of non-uniform control flow
+ // and not the if statement with the uniform condition.
+ std::string src = R"(
+@group(0) @binding(1) var<storage, read_write> non_uniform_value : vec4<f32>;
+
+fn foo(limit : ptr<function, f32>) -> f32 {
+ var i : i32;
+ if (f32(i) > *limit) {
+ return 0.0;
+ }
+ return 1.0f;
+}
+
+fn main() {
+ var param : f32 = non_uniform_value.y;
+ let i = foo(¶m);
+ let y = dpdx(vec3<f32>());
+}
+)";
+
+ RunTest(src, false);
+ EXPECT_EQ(error_,
+ R"(test:15:11 error: 'dpdx' must only be called from uniform control flow
+ let y = dpdx(vec3<f32>());
+ ^^^^
+
+test:14:15 note: non-uniform function call argument causes subsequent control flow to be non-uniform
+ let i = foo(¶m);
+ ^
+
+test:6:3 note: control flow depends on non-uniform value
+ if (f32(i) > *limit) {
+ ^^
+
+test:6:14 note: result of expression may be non-uniform
+ if (f32(i) > *limit) {
+ ^
+
+test:13:21 note: reading from read_write storage buffer 'non_uniform_value' may result in a non-uniform value
+ var param : f32 = non_uniform_value.y;
+ ^^^^^^^^^^^^^^^^^
+)");
+}
+
+TEST_F(UniformityAnalysisTest,
+ Error_ParameterRequiredToBeUniformForSubsequentControlFlow_ViaPointer_InLoop) {
+ // Make sure we correctly identify the function call as the source of non-uniform control flow
+ // and not the if statement with the uniform condition.
+ std::string src = R"(
+@group(0) @binding(1) var<storage, read_write> non_uniform_value : vec4<f32>;
+
+fn foo(limit : ptr<function, f32>) -> f32 {
+ var i : i32;
+ loop {
+ if (f32(i) > *limit) {
+ return 0.0;
+ }
+ }
+}
+
+fn main() {
+ var param : f32 = non_uniform_value.y;
+ let i = foo(¶m);
+ let y = dpdx(vec3<f32>());
+}
+)";
+
+ RunTest(src, false);
+ EXPECT_EQ(error_,
+ R"(test:16:11 error: 'dpdx' must only be called from uniform control flow
+ let y = dpdx(vec3<f32>());
+ ^^^^
+
+test:15:15 note: non-uniform function call argument causes subsequent control flow to be non-uniform
+ let i = foo(¶m);
+ ^
+
+test:7:5 note: control flow depends on non-uniform value
+ if (f32(i) > *limit) {
+ ^^
+
+test:4:8 note: reading from 'limit' may result in a non-uniform value
+fn foo(limit : ptr<function, f32>) -> f32 {
+ ^^^^^
+
+test:14:21 note: reading from read_write storage buffer 'non_uniform_value' may result in a non-uniform value
+ var param : f32 = non_uniform_value.y;
+ ^^^^^^^^^^^^^^^^^
+)");
+}
+
TEST_F(UniformityAnalysisTest, Error_ShortCircuitingExprCausesNonUniformControlFlow) {
// Make sure we correctly identify the short-circuit as the source of non-uniform control flow
// and not the if statement with the uniform condition.
