[Mesa-dev] [PATCH 2/4] glsl: Fix loop analysis of nested loops.

Paul Berry stereotype441 at gmail.com
Thu Nov 28 11:41:07 PST 2013


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".
---
 src/glsl/loop_analysis.cpp | 34 +++++++++++++++++++++-------------
 src/glsl/loop_analysis.h   | 12 ++++++++----
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/src/glsl/loop_analysis.cpp b/src/glsl/loop_analysis.cpp
index 3f2a14b..4921b81 100644
--- 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) {
@@ -240,14 +242,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;
 }
@@ -327,7 +334,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
@@ -367,9 +374,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 assignmnet 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 edad8f3..a0c174e 100644
--- a/src/glsl/loop_analysis.h
+++ b/src/glsl/loop_analysis.h
@@ -172,8 +172,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;
@@ -202,7 +205,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);
 
@@ -219,7 +222,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);
 };
 
-- 
1.8.4.2



More information about the mesa-dev mailing list