glsl: Fix loop analysis of nested loops.

Previously, when visiting a variable dereference, loop analysis would
only consider its effect on the innermost enclosing loop.  As a
result, when encountering a loop like this:

    for (int i = 0; i < 3; i++) {
      for (int j = 0; j < 3; j++) {
        ...
        i = 2;
      }
    }

it would incorrectly conclude that the outer loop ran three times.

Fixes piglit test "vs-inner-loop-modifies-outer-loop-var.shader_test".

Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

diff --git a/src/glsl/loop_analysis.cpp b/src/glsl/loop_analysis.cpp
index 9f1b1d2b1f95ef04e0726fea837c5167b64799a2..b423ad4a9c59e699041d92b34e627b08583dbe9c 100644 (file)
--- a/src/glsl/loop_analysis.cpp
+++ b/src/glsl/loop_analysis.cpp
@@ -38,20 +38,22 @@ static ir_rvalue *get_basic_induction_increment(ir_assignment *, hash_table *);
  *
  * \arg in_assignee is true if the reference was on the LHS of an assignment.
  *
- * \arg in_conditional_code is true if the reference occurred inside an if
- * statement.
+ * \arg in_conditional_code_or_nested_loop is true if the reference occurred
+ * inside an if statement or a nested loop.
  *
  * \arg current_assignment is the ir_assignment node that the loop variable is
  * on the LHS of, if any (ignored if \c in_assignee is false).
  */
 void
-loop_variable::record_reference(bool in_assignee, bool in_conditional_code,
+loop_variable::record_reference(bool in_assignee,
+                                bool in_conditional_code_or_nested_loop,
                                 ir_assignment *current_assignment)
 {
    if (in_assignee) {
       assert(current_assignment != NULL);
 
-      this->conditional_assignment = in_conditional_code
+      this->conditional_or_nested_assignment =
+         in_conditional_code_or_nested_loop
          || current_assignment->condition != NULL;
 
       if (this->first_assignment == NULL) {
@@ -241,14 +243,19 @@ loop_analysis::visit(ir_dereference_variable *ir)
    if (this->state.is_empty())
       return visit_continue;
 
-   loop_variable_state *const ls =
-      (loop_variable_state *) this->state.get_head();
+   bool nested = false;
 
-   ir_variable *var = ir->variable_referenced();
-   loop_variable *lv = ls->get_or_insert(var, this->in_assignee);
+   foreach_list(node, &this->state) {
+      loop_variable_state *const ls = (loop_variable_state *) node;
 
-   lv->record_reference(this->in_assignee, this->if_statement_depth > 0,
-                        this->current_assignment);
+      ir_variable *var = ir->variable_referenced();
+      loop_variable *lv = ls->get_or_insert(var, this->in_assignee);
+
+      lv->record_reference(this->in_assignee,
+                           nested || this->if_statement_depth > 0,
+                           this->current_assignment);
+      nested = true;
+   }
 
    return visit_continue;
 }
@@ -328,7 +335,7 @@ loop_analysis::visit_leave(ir_loop *ir)
       foreach_list_safe(node, &ls->variables) {
         loop_variable *lv = (loop_variable *) node;
 
-        if (lv->conditional_assignment || (lv->num_assignments > 1))
+        if (lv->conditional_or_nested_assignment || (lv->num_assignments > 1))
            continue;
 
         /* Process the RHS of the assignment.  If all of the variables
@@ -368,9 +375,10 @@ loop_analysis::visit_leave(ir_loop *ir)
       assert(lv->num_assignments == 1);
       assert(lv->first_assignment != NULL);
 
-      /* The assignmnet to the variable in the loop must be unconditional.
+      /* The assignment to the variable in the loop must be unconditional and
+       * not inside a nested loop.
        */
-      if (lv->conditional_assignment)
+      if (lv->conditional_or_nested_assignment)
         continue;
 
       /* Basic loop induction variables have a single assignment in the loop

diff --git a/src/glsl/loop_analysis.h b/src/glsl/loop_analysis.h
index 4fae829c3ab3663011f25dbf356ec2634269d403..3c3719b919e02c3f3068929fa8f7b84e63336119 100644 (file)
--- a/src/glsl/loop_analysis.h
+++ b/src/glsl/loop_analysis.h
@@ -167,8 +167,11 @@ public:
    /** Are all variables in the RHS of the assignment loop constants? */
    bool rhs_clean;
 
-   /** Is there an assignment to the variable that is conditional? */
-   bool conditional_assignment;
+   /**
+    * Is there an assignment to the variable that is conditional, or inside a
+    * nested loop?
+    */
+   bool conditional_or_nested_assignment;
 
    /** Reference to the first assignment to the variable in the loop body. */
    ir_assignment *first_assignment;
@@ -197,7 +200,7 @@ public:
    {
       const bool is_const = (this->num_assignments == 0)
         || ((this->num_assignments == 1)
-            && !this->conditional_assignment
+            && !this->conditional_or_nested_assignment
             && !this->read_before_write
             && this->rhs_clean);
 
@@ -214,7 +217,8 @@ public:
       return is_const;
    }
 
-   void record_reference(bool in_assignee, bool in_conditional_code,
+   void record_reference(bool in_assignee,
+                         bool in_conditional_code_or_nested_loop,
                          ir_assignment *current_assignment);
 };