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 <eric@anholt.net>

diff --git a/src/compiler/glsl/opt_constant_propagation.cpp b/src/compiler/glsl/opt_constant_propagation.cpp
index f00c9a1ce9630a3e1e573d6ae0f5a5486ef11114..22f2867e1214ca0351ac7567b6ac97b61786727b 100644 (file)
--- 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;