[uniformity] Fix ICE for pointer parameters
The code for determining the source of uniformity was assuming that a
parameter node was always visited directly from an identifier
expression node. This is not the case when there is control flow
before the identifier expression that introduces additional nodes for
merging values. Add extra metadata to parameter nodes so that we can
correctly emit their source info instead.
This also slightly improves error messages for pointer parameters.
Fixed: 42251307
Change-Id: I4a3fed2c5f9c177c0b227a8354fd1564599c8d0a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/186540
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/tint/lang/wgsl/resolver/uniformity.cc b/src/tint/lang/wgsl/resolver/uniformity.cc
index eb3c810..776d424 100644
--- a/src/tint/lang/wgsl/resolver/uniformity.cc
+++ b/src/tint/lang/wgsl/resolver/uniformity.cc
@@ -130,6 +130,7 @@
kFunctionCallArgumentContents,
kFunctionCallPointerArgumentResult,
kFunctionCallReturnValue,
+ kFunctionPointerParameterContents,
};
/// The type of the node.
@@ -214,13 +215,15 @@
parameters[i].sem = sem;
parameters[i].value = CreateNode({"param_", param_name});
+ parameters[i].value->ast = param;
if (sem->Type()->Is<core::type::Pointer>()) {
// Create extra nodes for a pointer parameter's initial contents and its contents
// when the function returns.
parameters[i].ptr_input_contents =
- CreateNode({"ptrparam_", param_name, "_input_contents"});
+ CreateNode({"ptrparam_", param_name, "_input_contents"}, param);
parameters[i].ptr_output_contents =
CreateNode({"ptrparam_", param_name, "_output_contents"});
+ parameters[i].ptr_input_contents->type = Node::kFunctionPointerParameterContents;
variables.Set(sem, parameters[i].ptr_input_contents);
local_var_decls.Add(sem);
} else {
@@ -1822,7 +1825,10 @@
Traverse(required_to_be_uniform);
// Get the source of the non-uniform value.
- auto* non_uniform_source = may_be_non_uniform->visited_from;
+ auto* non_uniform_source = may_be_non_uniform;
+ if (non_uniform_source == function.may_be_non_uniform) {
+ non_uniform_source = non_uniform_source->visited_from;
+ }
TINT_ASSERT(non_uniform_source);
// Show where the non-uniform value results in non-uniform control flow.
@@ -1859,11 +1865,10 @@
return "";
}
};
- auto param_type = [&](const sem::Parameter* param) {
- if (ast::HasAttribute<ast::BuiltinAttribute>(param->Declaration()->attributes)) {
+ auto param_type = [&](const ast::Parameter* param) {
+ if (ast::HasAttribute<ast::BuiltinAttribute>(param->attributes)) {
return "builtin ";
- } else if (ast::HasAttribute<ast::LocationAttribute>(
- param->Declaration()->attributes)) {
+ } else if (ast::HasAttribute<ast::LocationAttribute>(param->attributes)) {
return "user-defined input ";
} else {
return "parameter ";
@@ -1878,14 +1883,27 @@
if (auto* param = var->As<sem::Parameter>()) {
auto* func = param->Owner()->As<sem::Function>();
diagnostics_.AddNote(ident->source)
- << param_type(param) << "'" << NameFor(ident) << "' of '" << NameFor(func)
- << "' may be non-uniform";
+ << param_type(param->Declaration()) << "'" << NameFor(ident) << "' of '"
+ << NameFor(func) << "' may be non-uniform";
} else {
diagnostics_.AddNote(ident->source)
<< "reading from " << var_type(var) << "'" << NameFor(ident)
<< "' may result in a non-uniform value";
}
},
+ [&](const ast::Parameter* p) {
+ auto* param = sem_.Get(p);
+ auto* func = param->Owner()->As<sem::Function>();
+ if (non_uniform_source->type == Node::kFunctionPointerParameterContents) {
+ diagnostics_.AddNote(p->source)
+ << "parameter '" << NameFor(p) << "' of '" << NameFor(func)
+ << "' may point to a non-uniform value";
+ } else {
+ diagnostics_.AddNote(p->source)
+ << param_type(p) << "'" << NameFor(p) << "' of '" << NameFor(func)
+ << "' may be non-uniform";
+ }
+ },
[&](const ast::Variable* v) {
auto* var = sem_.Get(v);
diagnostics_.AddNote(v->source)
diff --git a/src/tint/lang/wgsl/resolver/uniformity_test.cc b/src/tint/lang/wgsl/resolver/uniformity_test.cc
index c4a5000..9c15fa6 100644
--- a/src/tint/lang/wgsl/resolver/uniformity_test.cc
+++ b/src/tint/lang/wgsl/resolver/uniformity_test.cc
@@ -433,9 +433,9 @@
if (i == 0) {
^^
-test:5:7 note: parameter 'i' of 'foo' may be non-uniform
- if (i == 0) {
- ^
+test:4:8 note: parameter 'i' of 'foo' may be non-uniform
+fn foo(i : i32) {
+ ^
test:11:7 note: possibly non-uniform value passed here
foo(rw);
@@ -3744,8 +3744,8 @@
if (*p == 0) {
^^
-test:5:8 note: parameter 'p' of 'bar' may be non-uniform
- if (*p == 0) {
+test:4:8 note: parameter 'p' of 'bar' may point to a non-uniform value
+fn bar(p : ptr<function, i32>) {
^
test:12:7 note: possibly non-uniform value passed via pointer here
@@ -3875,8 +3875,8 @@
if (*p == 0) {
^^
-test:5:8 note: parameter 'p' of 'bar' may be non-uniform
- if (*p == 0) {
+test:4:8 note: parameter 'p' of 'bar' may point to a non-uniform value
+fn bar(p : ptr<function, i32>) {
^
test:13:7 note: possibly non-uniform value passed via pointer here
@@ -3916,8 +3916,8 @@
if (*p == 0) {
^^
-test:5:8 note: parameter 'p' of 'bar' may be non-uniform
- if (*p == 0) {
+test:4:8 note: parameter 'p' of 'bar' may point to a non-uniform value
+fn bar(p : ptr<function, i32>) {
^
test:12:7 note: possibly non-uniform value passed via pointer here
@@ -4074,8 +4074,8 @@
if (*p == 0) {
^^
-test:5:8 note: parameter 'p' of 'zoo' may be non-uniform
- if (*p == 0) {
+test:4:8 note: parameter 'p' of 'zoo' may point to a non-uniform value
+fn zoo(p : ptr<function, i32>) {
^
test:11:7 note: possibly non-uniform value passed via pointer here
@@ -4162,8 +4162,8 @@
if (*p == 0) {
^^
-test:6:8 note: parameter 'p' of 'zoo' may be non-uniform
- if (*p == 0) {
+test:5:8 note: parameter 'p' of 'zoo' may point to a non-uniform value
+fn zoo(p : ptr<function, i32>) {
^
test:12:7 note: possibly non-uniform value passed via pointer here
@@ -8032,8 +8032,8 @@
if (*p == 0) {
^^
-test:7:8 note: parameter 'p' of 'bar' may be non-uniform
- if (*p == 0) {
+test:6:8 note: parameter 'p' of 'bar' may point to a non-uniform value
+fn bar(p : ptr<function, i32>) -> i32 {
^
test:15:9 note: possibly non-uniform value passed via pointer here
@@ -8108,8 +8108,8 @@
if (*p == 0) {
^^
-test:7:8 note: parameter 'p' of 'bar' may be non-uniform
- if (*p == 0) {
+test:6:8 note: parameter 'p' of 'bar' may point to a non-uniform value
+fn bar(p : ptr<function, i32>) -> i32 {
^
test:15:9 note: possibly non-uniform value passed via pointer here
@@ -8181,9 +8181,9 @@
if (*p == 0) {
^^
-test:10:8 note: parameter 'p' of 'b' may be non-uniform
- if (*p == 0) {
- ^
+test:9:6 note: parameter 'p' of 'b' may point to a non-uniform value
+fn b(p : ptr<function, i32>) -> i32 {
+ ^
test:19:22 note: possibly non-uniform value passed via pointer here
arr[a(&i)] = arr[b(&i)];
@@ -8222,9 +8222,9 @@
if (cond == 0) {
^^
-test:5:7 note: parameter 'cond' of 'bar' may be non-uniform
- if (cond == 0) {
- ^^^^
+test:4:8 note: parameter 'cond' of 'bar' may be non-uniform
+fn bar(cond : i32) -> i32 {
+ ^^^^
test:13:11 note: possibly non-uniform value passed here
arr[bar(non_uniform)] = 0;
@@ -8263,9 +8263,9 @@
if (cond == 0) {
^^
-test:5:7 note: parameter 'cond' of 'bar' may be non-uniform
- if (cond == 0) {
- ^^^^
+test:4:8 note: parameter 'cond' of 'bar' may be non-uniform
+fn bar(cond : i32) -> i32 {
+ ^^^^
test:13:14 note: possibly non-uniform value passed here
*&(arr[bar(non_uniform)]) = 0;
@@ -8304,9 +8304,9 @@
if (cond == 0) {
^^
-test:5:7 note: parameter 'cond' of 'bar' may be non-uniform
- if (cond == 0) {
- ^^^^
+test:4:8 note: parameter 'cond' of 'bar' may be non-uniform
+fn bar(cond : i32) -> i32 {
+ ^^^^
test:13:14 note: possibly non-uniform value passed here
(&arr)[bar(non_uniform)] = 0;
@@ -8345,9 +8345,9 @@
if (cond == 0) {
^^
-test:5:7 note: parameter 'cond' of 'bar' may be non-uniform
- if (cond == 0) {
- ^^^^
+test:4:8 note: parameter 'cond' of 'bar' may be non-uniform
+fn bar(cond : i32) -> i32 {
+ ^^^^
test:13:14 note: possibly non-uniform value passed here
(&(arr[bar(non_uniform)])).y = 0;
@@ -8418,9 +8418,9 @@
if (*p == 0) {
^^
-test:10:8 note: parameter 'p' of 'b' may be non-uniform
- if (*p == 0) {
- ^
+test:9:6 note: parameter 'p' of 'b' may point to a non-uniform value
+fn b(p : ptr<function, i32>) -> i32 {
+ ^
test:19:23 note: possibly non-uniform value passed via pointer here
arr[a(&i)] += arr[b(&i)];
@@ -9988,9 +9988,9 @@
if (a == 42) {
^^
-test:5:7 note: parameter 'a' of 'zoo' may be non-uniform
- if (a == 42) {
- ^
+test:4:8 note: parameter 'a' of 'zoo' may be non-uniform
+fn zoo(a : i32) {
+ ^
test:11:7 note: possibly non-uniform value passed here
zoo(b);
@@ -10096,5 +10096,52 @@
)");
}
+TEST_F(UniformityAnalysisTest, Error_PointerParameterContentsRequiresUniformity_AfterControlFlow) {
+ // Test that we can find the correct source of uniformity inside a function called with a
+ // pointer parameter, when the pointer contents is used after control flow that introduces extra
+ // nodes for merging the pointer contents.
+ std::string src = R"(
+var<private> non_uniform : i32;
+
+fn foo(p : ptr<function, i32>) {
+ for (var i = 0; i < 3; i++) {
+ continue;
+ }
+ if (*p == 0) {
+ return;
+ }
+ _ = dpdx(1.0);
+}
+
+fn main() {
+ var f = non_uniform;
+ foo(&f);
+}
+)";
+
+ RunTest(src, false);
+ EXPECT_EQ(error_,
+ R"(test:11:7 error: 'dpdx' must only be called from uniform control flow
+ _ = dpdx(1.0);
+ ^^^^^^^^^
+
+test:8:3 note: control flow depends on possibly non-uniform value
+ if (*p == 0) {
+ ^^
+
+test:4:8 note: parameter 'p' of 'foo' may point to a non-uniform value
+fn foo(p : ptr<function, i32>) {
+ ^
+
+test:16:7 note: possibly non-uniform value passed via pointer here
+ foo(&f);
+ ^^
+
+test:15:11 note: reading from module-scope private variable 'non_uniform' may result in a non-uniform value
+ var f = non_uniform;
+ ^^^^^^^^^^^
+)");
+}
+
} // namespace
} // namespace tint::resolver
