[Mesa-dev] [PATCH 03/10] glsl: don't let an 'if' then-branch kill copy propagation for else-branch

[Caio Marcelo de Oliveira Filho]

When handling 'if' in copy 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.

    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.

Results for Skylake:

  total instructions in shared programs: 15238463 -> 15238503 (<.01%)
  instructions in affected programs: 10317 -> 10357 (0.39%)
  helped: 0
  HURT: 20

  total cycles in shared programs: 571868000 -> 571868028 (<.01%)
  cycles in affected programs: 43507 -> 43535 (0.06%)
  helped: 14
  HURT: 6

The hurt instruction count is caused because the extra propagation
causes an input variable to be read from two branches of an
if (load_input intrinsic in NIR). Depending on the complexity of each
branch this might be a win or not in terms of cycles.
---
 src/compiler/glsl/opt_copy_propagation.cpp | 44 ++++++++++++----------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/src/compiler/glsl/opt_copy_propagation.cpp b/src/compiler/glsl/opt_copy_propagation.cpp
index 206dffe4f1c..4ba5eedb2b1 100644
--- a/src/compiler/glsl/opt_copy_propagation.cpp
+++ b/src/compiler/glsl/opt_copy_propagation.cpp
@@ -71,7 +71,7 @@ public:
 
    void add_copy(ir_assignment *ir);
    void kill(ir_variable *ir);
-   void handle_if_block(exec_list *instructions);
+   void handle_if_block(exec_list *instructions, set *kills, bool *killed_all);
 
    /** Hash of lhs->rhs: The available copies to propagate */
    hash_table *acp;
@@ -207,14 +207,13 @@ ir_copy_propagation_visitor::visit_enter(ir_call *ir)
 }
 
 void
-ir_copy_propagation_visitor::handle_if_block(exec_list *instructions)
+ir_copy_propagation_visitor::handle_if_block(exec_list *instructions, set *kills, bool *killed_all)
 {
    hash_table *orig_acp = this->acp;
    set *orig_kills = this->kills;
    bool orig_killed_all = this->killed_all;
 
-   kills = _mesa_set_create(NULL, _mesa_hash_pointer,
-                            _mesa_key_pointer_equal);
+   this->kills = kills;
    this->killed_all = false;
 
    /* Populate the initial acp with a copy of the original */
@@ -222,22 +221,12 @@ ir_copy_propagation_visitor::handle_if_block(exec_list *instructions)
 
    visit_list_elements(this, instructions);
 
-   if (this->killed_all) {
-      _mesa_hash_table_clear(orig_acp, NULL);
-   }
+   _mesa_hash_table_destroy(acp, NULL);
+   *killed_all = this->killed_all;
 
-   set *new_kills = this->kills;
    this->kills = orig_kills;
-   _mesa_hash_table_destroy(acp, NULL);
    this->acp = orig_acp;
-   this->killed_all = this->killed_all || orig_killed_all;
-
-   struct set_entry *s_entry;
-   set_foreach(new_kills, s_entry) {
-      kill((ir_variable *) s_entry->key);
-   }
-
-   _mesa_set_destroy(new_kills, NULL);
+   this->killed_all = orig_killed_all;
 }
 
 ir_visitor_status
@@ -245,8 +234,25 @@ ir_copy_propagation_visitor::visit_enter(ir_if *ir)
 {
    ir->condition->accept(this);
 
-   handle_if_block(&ir->then_instructions);
-   handle_if_block(&ir->else_instructions);
+   set *new_kills = _mesa_set_create(NULL, _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) {
+      _mesa_hash_table_clear(acp, NULL);
+      killed_all = true;
+   } else {
+      struct set_entry *s_entry;
+      set_foreach(new_kills, s_entry) {
+         kill((ir_variable *) s_entry->key);
+      }
+   }
+
+   _mesa_set_destroy(new_kills, NULL);
 
    /* handle_if_block() already descended into the children. */
    return visit_continue_with_parent;
-- 
2.17.1