glsl: Modify strategy for accumulating conditions when lowering if-statements

Previously if-statements were lowered from inner-most to outer-most
(i.e., bottom-up).  All assignments within an if-statement would have
the condition of the if-statement appended to its existing condition.
As a result the assignments from a deeply nested if-statement would
have a very long and complex condition.

Several shaders in the OpenGL ES2 conformance test suite contain
non-constant array indexing that has been lowered by the shader
writer.  These tests usually look something like:

    if (i == 0) {
        value = array[0];
    } else if (i == 1) {
        value = array[1];
    } else ...

The IR for the last assignment ends up as:

    (assign (expression bool && (expression bool ! (var_ref if_to_cond_assign_condition) ) (expression bool && (expression bool ! (var_ref if_to_cond_assign_condition@20) ) (expression bool && (expression bool ! (var_ref if_to_cond_assign_condition@22) ) (expression bool && (expression bool ! (var_ref if_to_cond_assign_condition@24) ) (var_ref if_to_cond_assign_condition@26) ) ) ) )  (x) (var_ref value) (array_ref (var_ref array) (constant int (5)))

The Mesa IR that is generated from this is just as awesome as you
might expect.

Three changes are made to the way if-statements are lowered.

1. Two condition variables, if_to_cond_assign_then and
if_to_cond_assign_else, are created for each if-then-else structure.
The former contains the "positive" condition, and the later contains
the "negative" condtion.  This change was implemented in the previous
patch.

2. Each condition variable is added to a hash-table when it is created.

3. When lowering an if-statement, assignments to existing condtion
variables get the current condition anded.  This ensures that nested
condition variables are only set to true when the condition variable
for all outer if-statements is also true.

Changes #1 and #3 combine to ensure the correctness of the resulting
code.

4. When a condition assignment is encountered with a condition that is
a dereference of a previously added condition variable, the condition
is not modified.

Change #4 prevents the continuous accumulation of conditions on
assignments.

If the original if-statements were:

    if (x) {
        if (a && b && c && d && e) {
            ...
        } else {
            ...
        }
    } else {
        if (g && h && i && j && k) {
            ...
        } else {
            ...
        }
    }

The lowered code will be

    if_to_cond_assign_then@1 = x;
    if_to_cond_assign_then@2 = a && b && c && d && e
        && if_to_cond_assign_then@1;
    ...
    if_to_cond_assign_else@2 = !if_to_cond_assign_then
        && if_to_cond_assign_then@1;
    ...

    if_to_cond_assign_else@1 = !if_to_cond_assign_then@1;
    if_to_cond_assign_then@3 = g && h && i && j;
        && if_to_cond_assign_else@1;
    ...
    if_to_cond_assign_else@3 = !if_to_cond_assign_then
        && if_to_cond_assign_else@1;
    ...

Depending on how instructions are emitted, there may be an extra
instruction due to the duplication of the '&&
if_to_cond_assign_{then,else}@1' on the nested else conditions.  In
addition, this may cause some unnecessary register pressure since in
the simple case (where the nested conditions are not complex) the
nested then-condition variables are live longer than strictly
necessary.

Before this change, one of the shaders in the OpenGL ES2 conformance
test suite's acos_float_frag_xvary generated 348 Mesa IR instructions.
After this change it only generates 124.  Many, but not all, of these
instructions would have also been eliminated by CSE.

Reviewed-by: Eric Anholt <eric@anholt.net>

1 files changed, 55 insertions, 11 deletions

diff --git a/src/glsl/lower_if_to_cond_assign.cpp b/src/glsl/lower_if_to_cond_assign.cpp
index a709755..94b24db 100644
--- a/src/glsl/lower_if_to_cond_assign.cpp
+++ b/src/glsl/lower_if_to_cond_assign.cpp
@@ -47,6 +47,7 @@
 
 #include "glsl_types.h"
 #include "ir.h"
+#include "program/hash_table.h"
 
 class ir_if_to_cond_assign_visitor : public ir_hierarchical_visitor {
 public:
@@ -55,6 +56,14 @@ public:
       this->progress = false;
       this->max_depth = max_depth;
       this->depth = 0;
+
+      this->condition_variables = hash_table_ctor(0, hash_table_pointer_hash,
+						  hash_table_pointer_compare);
+   }
+
+   ~ir_if_to_cond_assign_visitor()
+   {
+      hash_table_dtor(this->condition_variables);
    }
 
    ir_visitor_status visit_enter(ir_if *);
@@ -63,6 +72,8 @@ public:
    bool progress;
    unsigned max_depth;
    unsigned depth;
+
+   struct hash_table *condition_variables;
 };
 
 bool
@@ -95,7 +106,8 @@ check_control_flow(ir_instruction *ir, void *data)
 void
 move_block_to_cond_assign(void *mem_ctx,
 			  ir_if *if_ir, ir_rvalue *cond_expr,
-			  exec_list *instructions)
+			  exec_list *instructions,
+			  struct hash_table *ht)
 {
    foreach_list_safe(node, instructions) {
       ir_instruction *ir = (ir_instruction *) node;
@@ -103,14 +115,33 @@ move_block_to_cond_assign(void *mem_ctx,
       if (ir->ir_type == ir_type_assignment) {
 	 ir_assignment *assign = (ir_assignment *)ir;
 
-	 if (!assign->condition) {
-	    assign->condition = cond_expr->clone(mem_ctx, NULL);
-	 } else {
-	    assign->condition =
-	       new(mem_ctx) ir_expression(ir_binop_logic_and,
-					  glsl_type::bool_type,
-					  cond_expr->clone(mem_ctx, NULL),
-					  assign->condition);
+	 if (hash_table_find(ht, assign) == NULL) {
+	    hash_table_insert(ht, assign, assign);
+
+	    /* If the LHS of the assignment is a condition variable that was
+	     * previously added, insert an additional assignment of false to
+	     * the variable.
+	     */
+	    const bool assign_to_cv =
+	       hash_table_find(ht, assign->lhs->variable_referenced()) != NULL;
+
+	    if (!assign->condition) {
+	       if (assign_to_cv) {
+		  assign->rhs =
+		     new(mem_ctx) ir_expression(ir_binop_logic_and,
+						glsl_type::bool_type,
+						cond_expr->clone(mem_ctx, NULL),
+						assign->rhs);
+	       } else {
+		  assign->condition = cond_expr->clone(mem_ctx, NULL);
+	       }
+	    } else {
+	       assign->condition =
+		  new(mem_ctx) ir_expression(ir_binop_logic_and,
+					     glsl_type::bool_type,
+					     cond_expr->clone(mem_ctx, NULL),
+					     assign->condition);
+	    }
 	 }
       }
 
@@ -125,6 +156,7 @@ ir_if_to_cond_assign_visitor::visit_enter(ir_if *ir)
 {
    (void) ir;
    this->depth++;
+
    return visit_continue;
 }
 
@@ -170,7 +202,13 @@ ir_if_to_cond_assign_visitor::visit_leave(ir_if *ir)
    ir->insert_before(assign);
 
    move_block_to_cond_assign(mem_ctx, ir, then_cond,
-			     &ir->then_instructions);
+			     &ir->then_instructions,
+			     this->condition_variables);
+
+   /* Add the new condition variable to the hash table.  This allows us to
+    * find this variable when lowering other (enclosing) if-statements.
+    */
+   hash_table_insert(this->condition_variables, then_var, then_var);
 
    /* If there are instructions in the else-clause, store the inverse of the
     * condition to a variable.  Move all of the instructions from the
@@ -195,7 +233,13 @@ ir_if_to_cond_assign_visitor::visit_leave(ir_if *ir)
       ir->insert_before(assign);
 
       move_block_to_cond_assign(mem_ctx, ir, else_cond,
-				&ir->else_instructions);
+				&ir->else_instructions,
+				this->condition_variables);
+
+      /* Add the new condition variable to the hash table.  This allows us to
+       * find this variable when lowering other (enclosing) if-statements.
+       */
+      hash_table_insert(this->condition_variables, else_var, else_var);
    }
 
    ir->remove();