From 507a8037a731892c1b8cd6e9a8534d4a5447d5c5 Mon Sep 17 00:00:00 2001 From: Caio Marcelo de Oliveira Filho Date: Tue, 26 Jun 2018 16:26:46 -0700 Subject: glsl: don't let an 'if' then-branch kill copy propagation (elements) for else-branch When handling 'if' in copy propagation elements, if a certain variable was killed when processing the first branch of the 'if', then the second would get any propagation from previous nodes. x = y; if (...) { z = x; // This would turn into z = y. x = 22; // x gets killed. } else { w = x; // This would NOT turn into w = y. } With the change, we let copy propagation happen independently in the two branches and only then apply the killed values for the subsequent code. One example in shader-db part of shaders/unity/8.shader_test: (assign (xyz) (var_ref col_1) (var_ref tmpvar_8) ) (if (expression bool < (swiz y (var_ref xlv_TEXCOORD0) )(constant float (0.000000)) ) ( (assign (xyz) (var_ref col_1) (expression vec3 + (var_ref tmpvar_8) ... ) ... ) ) ( (assign (xyz) (var_ref col_1) (expression vec3 lrp (var_ref col_1) ... ) ... ) )) The variable col_1 was replaced by tmpvar_8 in the then-part but not in the else-part. NIR deals well with copy propagation, so it already covered for the missing ones that this patch fixes. Reviewed-by: Eric Anholt --- .../glsl/opt_copy_propagation_elements.cpp | 40 ++++++++++++---------- 1 file changed, 21 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/compiler/glsl/opt_copy_propagation_elements.cpp b/src/compiler/glsl/opt_copy_propagation_elements.cpp index 08ee63209ad..b5c90fff88d 100644 --- a/src/compiler/glsl/opt_copy_propagation_elements.cpp +++ b/src/compiler/glsl/opt_copy_propagation_elements.cpp @@ -257,7 +257,7 @@ public: void add_copy(ir_assignment *ir); void kill(kill_entry *k); - void handle_if_block(exec_list *instructions); + void handle_if_block(exec_list *instructions, exec_list *kills, bool *killed_all); copy_propagation_state *state; @@ -468,12 +468,12 @@ ir_copy_propagation_elements_visitor::visit_enter(ir_call *ir) } void -ir_copy_propagation_elements_visitor::handle_if_block(exec_list *instructions) +ir_copy_propagation_elements_visitor::handle_if_block(exec_list *instructions, exec_list *kills, bool *killed_all) { exec_list *orig_kills = this->kills; bool orig_killed_all = this->killed_all; - this->kills = new(mem_ctx) exec_list; + this->kills = kills; this->killed_all = false; /* Populate the initial acp with a copy of the original */ @@ -485,21 +485,9 @@ ir_copy_propagation_elements_visitor::handle_if_block(exec_list *instructions) delete this->state; this->state = orig_state; - if (this->killed_all) - this->state->erase_all(); - - exec_list *new_kills = this->kills; + *killed_all = this->killed_all; this->kills = orig_kills; - this->killed_all = this->killed_all || orig_killed_all; - - /* Move the new kills into the parent block's list, removing them - * from the parent's ACP list in the process. - */ - foreach_in_list_safe(kill_entry, k, new_kills) { - kill(k); - } - - ralloc_free(new_kills); + this->killed_all = orig_killed_all; } ir_visitor_status @@ -507,8 +495,22 @@ ir_copy_propagation_elements_visitor::visit_enter(ir_if *ir) { ir->condition->accept(this); - handle_if_block(&ir->then_instructions); - handle_if_block(&ir->else_instructions); + exec_list *new_kills = new(mem_ctx) exec_list; + bool then_killed_all = false; + bool else_killed_all = false; + + handle_if_block(&ir->then_instructions, new_kills, &then_killed_all); + handle_if_block(&ir->else_instructions, new_kills, &else_killed_all); + + if (then_killed_all || else_killed_all) { + state->erase_all(); + killed_all = true; + } else { + foreach_in_list_safe(kill_entry, k, new_kills) + kill(k); + } + + ralloc_free(new_kills); /* handle_if_block() already descended into the children. */ return visit_continue_with_parent; -- cgit v1.2.3