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>
This commit is contained in:
Antonio Maiorano 2022-10-20 17:15:34 +00:00 committed by Dawn LUCI CQ
parent bcf51bffa8
commit b69c6066c4
2 changed files with 131 additions and 25 deletions

View File

@ -831,7 +831,7 @@ class UniformityGraph {
// 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 @@ class UniformityGraph {
} 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 @@ class UniformityGraph {
// 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 @@ class UniformityGraph {
}
}
},
[&](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";
});

View File

@ -7962,6 +7962,99 @@ test:13:7 note: reading from read_write storage buffer 'non_uniform_value' may r
)");
}
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(&param);
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(&param);
^
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(&param);
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(&param);
^
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.