From 80e1c2f35f76562a6a62cfa630edaf97e6d6f227 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Sun, 4 Aug 2013 02:05:43 -0700 Subject: [PATCH] i965/fs: Optimize IF/MOV/ELSE/MOV/ENDIF to SEL when possible. Many GLSL shaders contain code of the form: x = condition ? foo : bar The compiler emits an ir_if tree for this, since each subexpression might be a complex tree that could have side-effects and short-circuit logic operations. However, the common case is to simply pick one of two constants or variable's values---which is exactly what SEL is for. Replacing IF/ELSE with SEL also simplifies the control flow graph, making optimization passes which work on basic blocks more effective. The shader-db statistics: total instructions in shared programs: 1655247 -> 1503234 (-9.18%) instructions in affected programs: 949188 -> 797175 (-16.02%) 2,970 shaders were helped, none hurt. Gained 181 SIMD16 programs. This helps Valve's Source Engine games (max -41.33%), The Cave (max -33.33%), Serious Sam 3 (max -18.64%), Yo Frankie! (max -30.19%), Zen Bound (max -22.22%), GStreamer (max -6.12%), and GLBenchmark 2.7 (max -1.94%). Signed-off-by: Kenneth Graunke Reviewed-by: Matt Turner --- src/mesa/drivers/dri/i965/brw_fs.h | 1 + src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 78 ++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h index 370ab6c3386..7feb2b680be 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.h +++ b/src/mesa/drivers/dri/i965/brw_fs.h @@ -369,6 +369,7 @@ public: fs_reg src0, fs_reg src1); bool try_emit_saturate(ir_expression *ir); bool try_emit_mad(ir_expression *ir, int mul_arg); + void try_replace_with_sel(); void emit_bool_to_cond_code(ir_rvalue *condition); void emit_if_gen6(ir_if *ir); void emit_unspill(fs_inst *inst, fs_reg reg, uint32_t spill_offset); diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp index ee7728cb6ec..a36c248550d 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp @@ -1842,6 +1842,82 @@ fs_visitor::emit_if_gen6(ir_if *ir) inst->predicate = BRW_PREDICATE_NORMAL; } +/** + * Try to replace IF/MOV/ELSE/MOV/ENDIF with SEL. + * + * Many GLSL shaders contain the following pattern: + * + * x = condition ? foo : bar + * + * The compiler emits an ir_if tree for this, since each subexpression might be + * a complex tree that could have side-effects or short-circuit logic. + * + * However, the common case is to simply select one of two constants or + * variable values---which is exactly what SEL is for. In this case, the + * assembly looks like: + * + * (+f0) IF + * MOV dst src0 + * ELSE + * MOV dst src1 + * ENDIF + * + * which can be easily translated into: + * + * (+f0) SEL dst src0 src1 + * + * If src0 is an immediate value, we promote it to a temporary GRF. + */ +void +fs_visitor::try_replace_with_sel() +{ + fs_inst *endif_inst = (fs_inst *) instructions.get_tail(); + assert(endif_inst->opcode == BRW_OPCODE_ENDIF); + + /* Pattern match in reverse: IF, MOV, ELSE, MOV, ENDIF. */ + int opcodes[] = { + BRW_OPCODE_IF, BRW_OPCODE_MOV, BRW_OPCODE_ELSE, BRW_OPCODE_MOV, + }; + + fs_inst *match = (fs_inst *) endif_inst->prev; + for (int i = 0; i < 4; i++) { + if (match->is_head_sentinel() || match->opcode != opcodes[4-i-1]) + return; + match = (fs_inst *) match->prev; + } + + /* The opcodes match; it looks like the right sequence of instructions. */ + fs_inst *else_mov = (fs_inst *) endif_inst->prev; + fs_inst *then_mov = (fs_inst *) else_mov->prev->prev; + fs_inst *if_inst = (fs_inst *) then_mov->prev; + + /* Check that the MOVs are the right form. */ + if (then_mov->dst.equals(else_mov->dst) && + !then_mov->is_partial_write() && + !else_mov->is_partial_write()) { + + /* Remove the matched instructions; we'll emit a SEL to replace them. */ + while (!if_inst->next->is_tail_sentinel()) + if_inst->next->remove(); + if_inst->remove(); + + /* Only the last source register can be a constant, so if the MOV in + * the "then" clause uses a constant, we need to put it in a temporary. + */ + fs_reg src0(then_mov->src[0]); + if (src0.file == IMM) { + src0 = fs_reg(this, glsl_type::float_type); + src0.type = then_mov->src[0].type; + emit(MOV(src0, then_mov->src[0])); + } + + fs_inst *sel = emit(BRW_OPCODE_SEL, then_mov->dst, src0, else_mov->src[0]); + sel->predicate = if_inst->predicate; + sel->predicate_inverse = if_inst->predicate_inverse; + sel->conditional_mod = if_inst->conditional_mod; + } +} + void fs_visitor::visit(ir_if *ir) { @@ -1881,6 +1957,8 @@ fs_visitor::visit(ir_if *ir) } emit(BRW_OPCODE_ENDIF); + + try_replace_with_sel(); } void