From: Paul Berry Date: Fri, 1 Jul 2011 19:36:23 +0000 (-0700) Subject: glsl: Use foreach_list in lower_jumps.cpp X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=382cee91a4bbbee45897239e6802ccaa5a5ad9c3;p=mesa.git glsl: Use foreach_list in lower_jumps.cpp 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, to make sure the altered IR is visited, 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. This results in sub-optimal output because lower_jumps.cpp produces a brand new set of temporary variables each time it is run, and the redundant temporary variables are not guaranteed to be eliminated by later optimization passes. 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. Reviewed-by: Kenneth Graunke --- diff --git a/src/glsl/lower_jumps.cpp b/src/glsl/lower_jumps.cpp index cbdd8eaa8be..199a0184fee 100644 --- a/src/glsl/lower_jumps.cpp +++ b/src/glsl/lower_jumps.cpp @@ -445,9 +445,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;