From 70f968d098c8cb914f4fc55766e1611513d5370e Mon Sep 17 00:00:00 2001
From: James Price <jrprice@google.com>
Date: Thu, 2 Feb 2023 02:28:27 +0000
Subject: [PATCH] tint: Fix masked ParameterRequiredToBeUniform

Reset the visited state of nodes before traversing to determine a
parameters requirements with respect to the function return value,
otherwise we may not capture a parameter->retval relationship
correctly.

Fixed: tint:1822
Change-Id: I9802a89fe1c8331b2f9dae594ccb045f339396fd
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/118080
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
---
 src/tint/resolver/uniformity.cc      |  2 ++
 src/tint/resolver/uniformity_test.cc | 40 ++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/src/tint/resolver/uniformity.cc b/src/tint/resolver/uniformity.cc
index 6c4b53a5ee..f5463a1198 100644
--- a/src/tint/resolver/uniformity.cc
+++ b/src/tint/resolver/uniformity.cc
@@ -474,6 +474,8 @@ class UniformityGraph {
 
         // If "Value_return" exists, look at which nodes are reachable from it.
         if (current_function_->value_return) {
+            current_function_->ResetVisited();
+
             utils::UniqueVector<Node*, 4> reachable;
             Traverse(current_function_->value_return, &reachable);
             if (reachable.Contains(current_function_->may_be_non_uniform)) {
diff --git a/src/tint/resolver/uniformity_test.cc b/src/tint/resolver/uniformity_test.cc
index 334352de4f..b8636f9b7d 100644
--- a/src/tint/resolver/uniformity_test.cc
+++ b/src/tint/resolver/uniformity_test.cc
@@ -8166,6 +8166,46 @@ test:6:9 note: return value of 'dpdx' may be non-uniform
 )");
 }
 
+TEST_F(UniformityAnalysisDiagnosticFilterTest,
+       ParameterRequiredToBeUniform_With_ParameterRequiredToBeUniformForReturnValue) {
+    // Make sure that both requirements on parameters are captured.
+    std::string src = R"(
+@diagnostic(info,derivative_uniformity)
+fn foo(x : bool) -> bool {
+  if (x) {
+    _ = dpdx(1.0); // Should trigger an info
+  }
+  return x;
+}
+
+var<private> non_uniform: bool;
+
+@diagnostic(error,derivative_uniformity)
+fn bar() {
+  let ret = foo(non_uniform);
+  if (ret) {
+    _ = dpdy(1.0); // Should trigger an error
+  }
+}
+
+)";
+
+    RunTest(src, false);
+    EXPECT_EQ(error_,
+              R"(test:16:9 error: 'dpdy' must only be called from uniform control flow
+    _ = dpdy(1.0); // Should trigger an error
+        ^^^^
+
+test:15:3 note: control flow depends on possibly non-uniform value
+  if (ret) {
+  ^^
+
+test:14:17 note: reading from module-scope private variable 'non_uniform' may result in a non-uniform value
+  let ret = foo(non_uniform);
+                ^^^^^^^^^^^
+)");
+}
+
 TEST_F(UniformityAnalysisDiagnosticFilterTest, BarriersNotAffected) {
     // Make sure that the diagnostic filter does not affect barriers.
     std::string src = R"(