glsl: dont create temps for builtin function inputs

It's not valid to be copying input variables to temps when
inlining atomic memory, interpolateAt functions, etc. We got away
with this previously because tree grafting would clean up the
mess but we shouldn't depend on an optimisation to clean up
invalid IR. Also I hope to remove tree grafting in a follow up
merge request.

Reviewed-by: Emma Anholt <emma@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19890>

diff --git a/src/compiler/glsl/opt_function_inlining.cpp b/src/compiler/glsl/opt_function_inlining.cpp
index d27944d913b9eeb80ac60d2eef0dfb705a02cba5..44a18cfd8d85157ba79327be7547174ab6711929 100644 (file)
--- a/src/compiler/glsl/opt_function_inlining.cpp
+++ b/src/compiler/glsl/opt_function_inlining.cpp
@@ -133,15 +133,31 @@ ir_save_lvalue_visitor::visit_enter(ir_dereference_array *deref)
 }
 
 static bool
-should_replace_variable(ir_variable *sig_param, ir_rvalue *param) {
+should_replace_variable(ir_variable *sig_param, ir_rvalue *param,
+                        bool is_builtin) {
+
+   if (sig_param->data.mode != ir_var_function_in &&
+       sig_param->data.mode != ir_var_const_in)
+      return false;
+
+   /* SSBO and shared vars might be passed to a built-in such as an atomic
+    * memory function, where copying these to a temp before passing to the
+    * atomic function is not valid so we must replace these instead. Also,
+    * shader inputs for interpolateAt funtions also need to be replaced.
+    *
+    * Our builtins should always use temps and not the inputs themselves to
+    * store temporay values so just checking is_builtin rather than string
+    * comparing the function name for e.g atomic* should always be safe.
+    */
+   if (is_builtin)
+      return true;
+
    /* For opaque types, we want the inlined variable references
     * referencing the passed in variable, since that will have
     * the location information, which an assignment of an opaque
     * variable wouldn't.
     */
-   return sig_param->type->contains_opaque() &&
-          param->is_dereference() &&
-          sig_param->data.mode == ir_var_function_in;
+   return sig_param->type->contains_opaque();
 }
 
 void
@@ -168,7 +184,8 @@ ir_call::generate_inline(ir_instruction *next_ir)
       ir_rvalue *param = (ir_rvalue *) actual_node;
 
       /* Generate a new variable for the parameter. */
-      if (should_replace_variable(sig_param, param)) {
+      if (should_replace_variable(sig_param, param,
+                                  this->callee->is_builtin())) {
          /* Actual replacement happens below */
         parameters[i] = NULL;
       } else {
@@ -251,7 +268,8 @@ ir_call::generate_inline(ir_instruction *next_ir)
       ir_rvalue *const param = (ir_rvalue *) actual_node;
       ir_variable *sig_param = (ir_variable *) formal_node;
 
-      if (should_replace_variable(sig_param, param)) {
+      if (should_replace_variable(sig_param, param,
+                                  this->callee->is_builtin())) {
          do_variable_replacement(&new_instructions, sig_param, param);
       }
    }

diff --git a/src/compiler/glsl/tests/lower_precision_test.py b/src/compiler/glsl/tests/lower_precision_test.py
index 999c6267af338a6d54a6f30679e9829bf938a023..21fc572187ff5741b24725a50f302fb9db333f9a 100644 (file)
--- a/src/compiler/glsl/tests/lower_precision_test.py
+++ b/src/compiler/glsl/tests/lower_precision_test.py
@@ -1268,7 +1268,7 @@ TESTS = [
                  color2 = y + 1;
          }
          """,
-         r'assign  \(x\) \(var_ref x@2\)  \(expression float f162f'),
+         r'assign  \(x\) \(var_ref compiler_temp@2\)  \(expression uint bitcast_f2u \(expression float f162f'),
     Test("ldexp",
          """
          #version 310 es
@@ -1301,7 +1301,7 @@ TESTS = [
                  color *= carry;
          }
          """,
-         r'expression uint \+ \(var_ref x\) \(var_ref y'),
+         r'expression uint \+ \(expression uint u2u \(expression uint16_t \* \(expression uint16_t u2ump \(var_ref x\) \) \(constant uint16_t \(2\)\) \) \) \(expression uint u2u \(expression uint16_t \* \(expression uint16_t u2ump \(var_ref y'),
     Test("usubBorrow",
          """
          #version 310 es
@@ -1318,7 +1318,7 @@ TESTS = [
                  color *= borrow;
          }
          """,
-         r'expression uint \- \(var_ref x\) \(var_ref y'),
+         r'expression uint \- \(expression uint u2u \(expression uint16_t \* \(expression uint16_t u2ump \(var_ref x\) \) \(constant uint16_t \(2\)\) \) \) \(expression uint u2u \(expression uint16_t \* \(expression uint16_t u2ump \(var_ref y'),
     Test("imulExtended",
          """
          #version 310 es