From 5880ed164abb36f816bbea1a715ad7bc988c3648 Mon Sep 17 00:00:00 2001
From: James Price <jrprice@google.com>
Date: Tue, 17 May 2022 08:08:44 +0000
Subject: [PATCH] tint: Fix edge for CallSiteRequiredToBeUniform

If the CallSiteRequiredToBeUniform tag is present, add the edge from
RequiredToBeUniform to a new diagnostic node for the function call,
instead of to the control flow coming out of the function call. Doing
the latter causes a false positive when a function both requires
uniform control flow and causes non-uniform control flow.

Bug: tint:880
Change-Id: Icade8f76302e8c21529502f5f945f1981acfc45a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/90582
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Auto-Submit: James Price <jrprice@google.com>
---
 src/tint/resolver/uniformity.cc      | 12 +++++++-----
 src/tint/resolver/uniformity_test.cc | 23 +++++++++++++++++++++++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/src/tint/resolver/uniformity.cc b/src/tint/resolver/uniformity.cc
index 60208f22f3..350e93bd06 100644
--- a/src/tint/resolver/uniformity.cc
+++ b/src/tint/resolver/uniformity.cc
@@ -1150,6 +1150,11 @@ class UniformityGraph {
             args.push_back(arg_node);
         }
 
+        // Note: This is an additional node that isn't described in the specification, for the
+        // purpose of providing diagnostic information.
+        Node* call_node = CreateNode(name + "_call", call);
+        call_node->AddEdge(cf_last_arg);
+
         Node* result = CreateNode(name + "_return_value", call);
         result->type = Node::kFunctionCallReturnValue;
         Node* cf_after = CreateNode("CF_after_" + name, call);
@@ -1199,12 +1204,9 @@ class UniformityGraph {
             });
 
         if (callsite_tag == CallSiteRequiredToBeUniform) {
-            // Note: This deviates from the rules in the specification, which would add the edge
-            // directly to the incoming CF node. Going through CF_after instead makes it easier to
-            // produce diagnostics that can identify the function being called.
-            current_function_->required_to_be_uniform->AddEdge(cf_after);
+            current_function_->required_to_be_uniform->AddEdge(call_node);
         }
-        cf_after->AddEdge(cf_last_arg);
+        cf_after->AddEdge(call_node);
 
         if (function_tag == SubsequentControlFlowMayBeNonUniform) {
             cf_after->AddEdge(current_function_->may_be_non_uniform);
diff --git a/src/tint/resolver/uniformity_test.cc b/src/tint/resolver/uniformity_test.cc
index c049666ef0..4a71045339 100644
--- a/src/tint/resolver/uniformity_test.cc
+++ b/src/tint/resolver/uniformity_test.cc
@@ -5584,6 +5584,29 @@ test:7:12 note: reading from read_write storage buffer 'rw' may result in a non-
 /// Miscellaneous statement and expression tests.
 ////////////////////////////////////////////////////////////////////////////////
 
+TEST_F(UniformityAnalysisTest, FunctionRequiresUniformFlowAndCausesNonUniformFlow) {
+    // Test that a function that requires uniform flow and then causes non-uniform flow can be
+    // called without error.
+    std::string src = R"(
+@group(0) @binding(0) var<storage, read_write> non_uniform_global : i32;
+
+fn foo() {
+  _ = dpdx(0.5);
+
+  if (non_uniform_global == 0) {
+    discard;
+  }
+}
+
+@stage(fragment)
+fn main() {
+  foo();
+}
+)";
+
+    RunTest(src, true);
+}
+
 TEST_F(UniformityAnalysisTest, TypeConstructor) {
     std::string src = R"(
 @group(0) @binding(0) var<storage, read_write> non_uniform_global : i32;