From d7849fd1dafafa3489e0be389dabb09502dae26e Mon Sep 17 00:00:00 2001 From: Caio Marcelo de Oliveira Filho Date: Fri, 15 Jun 2018 13:59:45 -0700 Subject: [PATCH] glsl: don't let an 'if' then-branch kill const propagation for else-branch When handling 'if' in constant propagation, if a certain variable was killed when processing the first branch of the 'if', then the second would get any propagation from previous nodes. This is similar to the change done for copy propagation code. x = 1; if (...) { z = x; // This would turn into z = 1. x = 22; // x gets killed. } else { w = x; // This would NOT turn into w = 1. } With the change, we let constant propagation happen independently in the two branches and only then apply the killed values for the subsequent code. The new code use a single hash table for keeping the kills of both branches (the branches only write to it), and it gets deleted after we use -- instead of waiting for mem_ctx to collect it. NIR deals well with constant propagation, so it already covered for the missing ones that this patch fixes. Reviewed-by: Eric Anholt --- .../glsl/opt_constant_propagation.cpp | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/compiler/glsl/opt_constant_propagation.cpp b/src/compiler/glsl/opt_constant_propagation.cpp index f00c9a1ce96..22f2867e121 100644 --- a/src/compiler/glsl/opt_constant_propagation.cpp +++ b/src/compiler/glsl/opt_constant_propagation.cpp @@ -105,7 +105,7 @@ public: void constant_folding(ir_rvalue **rvalue); void constant_propagation(ir_rvalue **rvalue); void kill(ir_variable *ir, unsigned write_mask); - void handle_if_block(exec_list *instructions); + void handle_if_block(exec_list *instructions, hash_table *kills, bool *killed_all); void handle_rvalue(ir_rvalue **rvalue); /** List of acp_entry: The available constants to propagate */ @@ -338,15 +338,14 @@ ir_constant_propagation_visitor::visit_enter(ir_call *ir) } void -ir_constant_propagation_visitor::handle_if_block(exec_list *instructions) +ir_constant_propagation_visitor::handle_if_block(exec_list *instructions, hash_table *kills, bool *killed_all) { exec_list *orig_acp = this->acp; hash_table *orig_kills = this->kills; bool orig_killed_all = this->killed_all; this->acp = new(mem_ctx) exec_list; - this->kills = _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer, - _mesa_key_pointer_equal); + this->kills = kills; this->killed_all = false; /* Populate the initial acp with a constant of the original */ @@ -356,18 +355,10 @@ ir_constant_propagation_visitor::handle_if_block(exec_list *instructions) visit_list_elements(this, instructions); - if (this->killed_all) { - orig_acp->make_empty(); - } - - hash_table *new_kills = this->kills; + *killed_all = this->killed_all; this->kills = orig_kills; this->acp = orig_acp; - this->killed_all = this->killed_all || orig_killed_all; - - hash_entry *htk; - hash_table_foreach(new_kills, htk) - kill((ir_variable *) htk->key, (uintptr_t) htk->data); + this->killed_all = orig_killed_all; } ir_visitor_status @@ -376,8 +367,24 @@ ir_constant_propagation_visitor::visit_enter(ir_if *ir) ir->condition->accept(this); handle_rvalue(&ir->condition); - handle_if_block(&ir->then_instructions); - handle_if_block(&ir->else_instructions); + hash_table *new_kills = _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer, + _mesa_key_pointer_equal); + 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) { + acp->make_empty(); + killed_all = true; + } else { + hash_entry *htk; + hash_table_foreach(new_kills, htk) + kill((ir_variable *) htk->key, (uintptr_t) htk->data); + } + + _mesa_hash_table_destroy(new_kills, NULL); /* handle_if_block() already descended into the children. */ return visit_continue_with_parent; -- 2.30.2