From 877db5a792df7f5971c1906a3677b066926b9832 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Thu, 28 Nov 2013 10:48:37 -0800 Subject: [PATCH] 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 Reviewed-by: Ian Romanick --- 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 9f1b1d2b1f9..b423ad4a9c5 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) { @@ -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 4fae829c3ab..3c3719b919e 100644 --- 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); }; -- 2.30.2