[Mesa-dev] [PATCH 2/2] glsl: Make copy propagation not panic when it sees an intrinsic.

[Kenneth Graunke]

A number of games have large arrays of constants, which we promote to
uniforms.  This introduces copies from the uniform array to the original
temporary array.  Normally, copy propagation eliminates those copies,
making everything refer to the uniform array directly.

A number of shaders in "Deus Ex: Mankind Divided" recently exposed a
limitation of copy propagation - if we had any intrinsics (i.e. image
access in a compute shader), we weren't able to get rid of these copies.

That meant that any variable indexing remained on the temporary array
rather being moved to the uniform array.  i965's scalar backend
currently doesn't support indirect addressing of temporary arrays,
which meant lowering it to if-ladders.  This was horrible.

According to Marek, on radeonsi/GCN, "F1 2015" uses 64% less
spilled-temp-array memory.

On i965/Skylake:

total instructions in shared programs: 13699698 -> 13654519 (-0.33%)
instructions in affected programs: 56046 -> 10867 (-80.61%)
helped: 14
HURT: 0

total cycles in shared programs: 278651496 -> 275422328 (-1.16%)
cycles in affected programs: 5791516 -> 2562348 (-55.76%)
helped: 14
HURT: 0

total spills in shared programs: 14990 -> 14685 (-2.03%)
spills in affected programs: 345 -> 40 (-88.41%)
helped: 9
HURT: 1

total fills in shared programs: 17264 -> 16744 (-3.01%)
fills in affected programs: 705 -> 185 (-73.76%)
helped: 9
HURT: 1

Helps Tomb Raider and Deus Ex: Mankind Divided.

Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>
---
 src/compiler/glsl/opt_copy_propagation.cpp | 31 ++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/src/compiler/glsl/opt_copy_propagation.cpp b/src/compiler/glsl/opt_copy_propagation.cpp
index 247c498..2240421 100644
--- a/src/compiler/glsl/opt_copy_propagation.cpp
+++ b/src/compiler/glsl/opt_copy_propagation.cpp
@@ -186,11 +186,34 @@ ir_copy_propagation_visitor::visit_enter(ir_call *ir)
       }
    }
 
-   /* Since we're unlinked, we don't (necessarily) know the side effects of
-    * this call.  So kill all copies.
+   /* Since this pass can run when unlinked, we don't (necessarily) know
+    * the side effects of calls.  (When linked, most calls are inlined
+    * anyway, so it doesn't matter much.)
+    *
+    * One place where this does matter is IR intrinsics.  They're never
+    * inlined.  We also know what they do - while some have side effects
+    * (such as image writes), none edit random global variables.  So we
+    * can assume they're side-effect free (other than the return value
+    * and out parameters).
     */
-   _mesa_hash_table_clear(acp, NULL);
-   this->killed_all = true;
+   if (!ir->callee->is_intrinsic()) {
+      _mesa_hash_table_clear(acp, NULL);
+      this->killed_all = true;
+   } else {
+      if (ir->return_deref)
+         kill(ir->return_deref->var);
+
+      foreach_two_lists(formal_node, &ir->callee->parameters,
+                        actual_node, &ir->actual_parameters) {
+         ir_variable *sig_param = (ir_variable *) formal_node;
+         if (sig_param->data.mode == ir_var_function_out ||
+             sig_param->data.mode == ir_var_function_inout) {
+            ir_rvalue *ir = (ir_rvalue *) actual_node;
+            ir_variable *var = ir->variable_referenced();
+            kill(var);
+         }
+      }
+   }
 
    return visit_continue_with_parent;
 }
-- 
2.10.2