[Mesa-dev] [PATCH 1/2] i965/fs: Avoid generating extra AND instructions on bool logic ops.

[Eric Anholt]

By making a bool fs_reg only have a defined low bit (matching CMP
output), instead of being a full 0 or 1 value, we reduce the ANDs
generated in logic chains like:

   if (v_texcoord.x < 0.0 || v_texcoord.x > texwidth ||
       v_texcoord.y < 0.0 || v_texcoord.y > 1.0)
      discard;

My concern originally when writing this code was that we would end up
generating unnecessary ANDs on bool uniforms, so I put the ANDs right
at the point of doing the CMPs that otherwise set only the low bit.
However, in order to use a bool, we're generating some instruction
anyway (e.g. moving it so as to produce a condition code update), and
those instructions can often be turned into an AND at that point.  It
turns out in the shaders I have on hand, none of them regress in
instruction count:

Total instructions: 212752 -> 212648
39/1246 programs affected (3.1%)
14519 -> 14415 instructions in affected programs (0.7% reduction)
---
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   36 ++++++++++----------------
 1 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 15eae43..eb129ce 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -381,7 +381,6 @@ fs_visitor::visit(ir_expression *ir)
 
       inst = emit(BRW_OPCODE_CMP, temp, op[0], op[1]);
       inst->conditional_mod = brw_conditional_for_comparison(ir->operation);
-      emit(BRW_OPCODE_AND, this->result, this->result, fs_reg(0x1));
       break;
 
    case ir_binop_logic_xor:
@@ -427,11 +426,19 @@ fs_visitor::visit(ir_expression *ir)
       break;
    case ir_unop_i2f:
    case ir_unop_u2f:
-   case ir_unop_b2f:
-   case ir_unop_b2i:
    case ir_unop_f2i:
       emit(BRW_OPCODE_MOV, this->result, op[0]);
       break;
+
+   case ir_unop_b2i:
+      inst = emit(BRW_OPCODE_AND, this->result, op[0], fs_reg(1));
+      break;
+   case ir_unop_b2f:
+      temp = fs_reg(this, glsl_type::int_type);
+      emit(BRW_OPCODE_AND, temp, op[0], fs_reg(1));
+      emit(BRW_OPCODE_MOV, this->result, temp);
+      break;
+
    case ir_unop_f2b:
    case ir_unop_i2b:
       temp = this->result;
@@ -443,7 +450,6 @@ fs_visitor::visit(ir_expression *ir)
 
       inst = emit(BRW_OPCODE_CMP, temp, op[0], fs_reg(0.0f));
       inst->conditional_mod = BRW_CONDITIONAL_NZ;
-      inst = emit(BRW_OPCODE_AND, this->result, this->result, fs_reg(1));
       break;
 
    case ir_unop_trunc:
@@ -1478,19 +1484,9 @@ fs_visitor::emit_bool_to_cond_code(ir_rvalue *ir)
 	 break;
 
       case ir_binop_logic_xor:
-	 inst = emit(BRW_OPCODE_XOR, reg_null_d, op[0], op[1]);
-	 inst->conditional_mod = BRW_CONDITIONAL_NZ;
-	 break;
-
       case ir_binop_logic_or:
-	 inst = emit(BRW_OPCODE_OR, reg_null_d, op[0], op[1]);
-	 inst->conditional_mod = BRW_CONDITIONAL_NZ;
-	 break;
-
       case ir_binop_logic_and:
-	 inst = emit(BRW_OPCODE_AND, reg_null_d, op[0], op[1]);
-	 inst->conditional_mod = BRW_CONDITIONAL_NZ;
-	 break;
+	 goto out;
 
       case ir_unop_f2b:
 	 if (intel->gen >= 6) {
@@ -1531,15 +1527,11 @@ fs_visitor::emit_bool_to_cond_code(ir_rvalue *ir)
       return;
    }
 
+out:
    ir->accept(this);
 
-   if (intel->gen >= 6) {
-      fs_inst *inst = emit(BRW_OPCODE_AND, reg_null_d, this->result, fs_reg(1));
-      inst->conditional_mod = BRW_CONDITIONAL_NZ;
-   } else {
-      fs_inst *inst = emit(BRW_OPCODE_MOV, reg_null_d, this->result);
-      inst->conditional_mod = BRW_CONDITIONAL_NZ;
-   }
+   fs_inst *inst = emit(BRW_OPCODE_AND, reg_null_d, this->result, fs_reg(1));
+   inst->conditional_mod = BRW_CONDITIONAL_NZ;
 }
 
 /**
-- 
1.7.9.1