[Mesa-dev] [PATCH 09/11] glsl: Use foreach_list in lower_jumps.cpp

[Paul Berry]

The visitor class in lower_jumps.cpp never removes or replaces the
instruction being visited, but it frequently alters or removes the
instructions that follow it.  Therefore, in order to be safe, it needs
to iterate through exec_lists using foreach_list rather than
visit_exec_list().

Without this patch, lower_jumps.cpp may require multiple passes in
order to lower all jumps, resulting in sub-optimal output.  Also,
certain invariants assumed by lower_jumps.cpp may fail to hold,
causing assertion failures.

Fixes unit tests test_lower_pulled_out_jump,
test_lower_unified_returns, test_lower_guarded_conditional_break,
test_lower_return_non_void_at_end_of_loop, and test_lower_returns_3.
---
 src/glsl/lower_jumps.cpp |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/src/glsl/lower_jumps.cpp b/src/glsl/lower_jumps.cpp
index 51a4cf0..103d64f 100644
--- a/src/glsl/lower_jumps.cpp
+++ b/src/glsl/lower_jumps.cpp
@@ -447,9 +447,20 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor {
 
    block_record visit_block(exec_list* list)
    {
+      /* Note: since visiting a node may change that node's next
+       * pointer, we can't use visit_exec_list(), because
+       * visit_exec_list() caches the node's next pointer before
+       * visiting it.  So we use foreach_list() instead.
+       *
+       * foreach_list() isn't safe if the node being visited gets
+       * removed, but fortunately this visitor doesn't do that.
+       */
+
       block_record saved_block = this->block;
       this->block = block_record();
-      visit_exec_list(list, this);
+      foreach_list(node, list) {
+         ((ir_instruction *) node)->accept(this);
+      }
       block_record ret = this->block;
       this->block = saved_block;
       return ret;
-- 
1.7.5.4