Mesa (master): glsl: compute lvalues of [in] out parameters before inlined function body

[Nicolai Hähnle]

Module: Mesa
Branch: master
Commit: 1ef505bb02b00a577c33e1af27d983abd14465e7
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=1ef505bb02b00a577c33e1af27d983abd14465e7

Author: Nicolai Hähnle <nicolai.haehnle at amd.com>
Date:   Wed Oct 19 19:43:39 2016 +0200

glsl: compute lvalues of [in]out parameters before inlined function body

This is required when an out argument involves an array index that is either
a global variable modified by the function or another out argument in the
same function call.

Fixes the shaders/out-parameter-indexing/vs-inout-index-inout-* tests.

v2:
- modify the ir_dereference_array nodes in place
- use ir_hierarchical_visitor
v3: use base_ir (Ian Romanick)

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

---

 src/compiler/glsl/opt_function_inlining.cpp | 91 +++++++++++++++++++++++++----
 1 file changed, 81 insertions(+), 10 deletions(-)

diff --git a/src/compiler/glsl/opt_function_inlining.cpp b/src/compiler/glsl/opt_function_inlining.cpp
index 83534bf..62c1f4b 100644
--- a/src/compiler/glsl/opt_function_inlining.cpp
+++ b/src/compiler/glsl/opt_function_inlining.cpp
@@ -62,6 +62,11 @@ public:
    bool progress;
 };
 
+class ir_save_lvalue_visitor : public ir_hierarchical_visitor {
+public:
+   virtual ir_visitor_status visit_enter(ir_dereference_array *);
+};
+
 } /* unnamed namespace */
 
 bool
@@ -95,6 +100,37 @@ replace_return_with_assignment(ir_instruction *ir, void *data)
    }
 }
 
+/* Save the given lvalue before the given instruction.
+ *
+ * This is done by adding temporary variables into which the current value
+ * of any array indices are saved, and then modifying the dereference chain
+ * in-place to point to those temporary variables.
+ *
+ * The hierarchical visitor is only used to traverse the left-hand-side chain
+ * of derefs.
+ */
+ir_visitor_status
+ir_save_lvalue_visitor::visit_enter(ir_dereference_array *deref)
+{
+   if (deref->array_index->ir_type != ir_type_constant) {
+      void *ctx = ralloc_parent(deref);
+      ir_variable *index;
+      ir_assignment *assignment;
+
+      index = new(ctx) ir_variable(deref->array_index->type, "saved_idx", ir_var_temporary);
+      base_ir->insert_before(index);
+
+      assignment = new(ctx) ir_assignment(new(ctx) ir_dereference_variable(index),
+                                          deref->array_index, 0);
+      base_ir->insert_before(assignment);
+
+      deref->array_index = new(ctx) ir_dereference_variable(index);
+   }
+
+   deref->array->accept(this);
+   return visit_stop;
+}
+
 void
 ir_call::generate_inline(ir_instruction *next_ir)
 {
@@ -139,15 +175,50 @@ ir_call::generate_inline(ir_instruction *next_ir)
 	 next_ir->insert_before(parameters[i]);
       }
 
-      /* Move the actual param into our param variable if it's an 'in' type. */
-      if (parameters[i] && (sig_param->data.mode == ir_var_function_in ||
-			    sig_param->data.mode == ir_var_const_in ||
-			    sig_param->data.mode == ir_var_function_inout)) {
-	 ir_assignment *assign;
-
-	 assign = new(ctx) ir_assignment(new(ctx) ir_dereference_variable(parameters[i]),
-					 param, NULL);
-	 next_ir->insert_before(assign);
+      /* Section 6.1.1 (Function Calling Conventions) of the OpenGL Shading
+       * Language 4.5 spec says:
+       *
+       *    "All arguments are evaluated at call time, exactly once, in order,
+       *     from left to right. [...] Evaluation of an out parameter results
+       *     in an l-value that is used to copy out a value when the function
+       *     returns."
+       *
+       * I.e., we have to take temporary copies of any relevant array indices
+       * before the function body is executed.
+       *
+       * This ensures that
+       * (a) if an array index expressions refers to a variable that is
+       *     modified by the execution of the function body, we use the
+       *     original value as intended, and
+       * (b) if an array index expression has side effects, those side effects
+       *     are only executed once and at the right time.
+       */
+      if (parameters[i]) {
+         if (sig_param->data.mode == ir_var_function_in ||
+             sig_param->data.mode == ir_var_const_in) {
+            ir_assignment *assign;
+
+            assign = new(ctx) ir_assignment(new(ctx) ir_dereference_variable(parameters[i]),
+                                            param, NULL);
+            next_ir->insert_before(assign);
+         } else {
+            assert(sig_param->data.mode == ir_var_function_out ||
+                   sig_param->data.mode == ir_var_function_inout);
+            assert(param->is_lvalue());
+
+            ir_save_lvalue_visitor v;
+            v.base_ir = next_ir;
+
+            param->accept(&v);
+
+            if (sig_param->data.mode == ir_var_function_inout) {
+               ir_assignment *assign;
+
+               assign = new(ctx) ir_assignment(new(ctx) ir_dereference_variable(parameters[i]),
+                                               param->clone(ctx, NULL)->as_rvalue(), NULL);
+               next_ir->insert_before(assign);
+            }
+         }
       }
 
       ++i;
@@ -196,7 +267,7 @@ ir_call::generate_inline(ir_instruction *next_ir)
 			    sig_param->data.mode == ir_var_function_inout)) {
 	 ir_assignment *assign;
 
-	 assign = new(ctx) ir_assignment(param->clone(ctx, NULL)->as_rvalue(),
+         assign = new(ctx) ir_assignment(param,
 					 new(ctx) ir_dereference_variable(parameters[i]),
 					 NULL);
 	 next_ir->insert_before(assign);