[Mesa-dev] [PATCH v2 09/11] glsl: don't let an 'if' then-branch kill copy propagation (elements) for else-branch

[Caio Marcelo de Oliveira Filho]

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 <e...@anholt.net>
---
 .../glsl/opt_copy_propagation_elements.cpp    | 40 ++++++++++---------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/compiler/glsl/opt_copy_propagation_elements.cpp 
b/src/compiler/glsl/opt_copy_propagation_elements.cpp
index 989bd81afae..d2c24697203 100644
--- a/src/compiler/glsl/opt_copy_propagation_elements.cpp
+++ b/src/compiler/glsl/opt_copy_propagation_elements.cpp
@@ -260,7 +260,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;
 
@@ -471,12 +471,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 */
@@ -488,21 +488,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
@@ -510,8 +498,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;
-- 
2.18.0

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev