From a352e2d08e0a141298275e77f25541218a97afb7 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Mon, 1 Aug 2011 14:13:10 -0700 Subject: [PATCH] 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 --- src/glsl/lower_if_to_cond_assign.cpp | 66 +++++++++++++++++++++++----- 1 file 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 a7097556c14..94b24db1b29 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(); -- 2.30.2