i965: Avoid generating MOVs for assignments of expressions.

No statistically significant difference measured in 3dbenchmark
egypt/pro.  It does reduce fragment shader instructions across
shader-db by 0.3%.

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 49a4c4dcb04818ecfde5c336086a50d57baccf5e..af70d922bac7ec8c00ea601e92b7147853db27b3 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -818,6 +818,7 @@ fs_visitor::try_emit_saturate(ir_expression *ir)
    if (!sat_val)
       return false;
 
+   this->result = reg_undef;
    sat_val->accept(this);
    fs_reg src = this->result;
 
@@ -864,7 +865,11 @@ fs_visitor::visit(ir_expression *ir)
    if (try_emit_saturate(ir))
       return;
 
+   /* This is where our caller would like us to put the result, if possible. */
+   fs_reg saved_result_storage = this->result;
+
    for (operand = 0; operand < ir->get_num_operands(); operand++) {
+      this->result = reg_undef;
       ir->operands[operand]->accept(this);
       if (this->result.file == BAD_FILE) {
         ir_print_visitor v;
@@ -882,10 +887,14 @@ fs_visitor::visit(ir_expression *ir)
       assert(!ir->operands[operand]->type->is_vector());
    }
 
-   /* Storage for our result.  If our result goes into an assignment, it will
-    * just get copy-propagated out, so no worries.
+   /* Inherit storage from our parent if possible, and otherwise we
+    * alloc a temporary.
     */
-   this->result = fs_reg(this, ir->type);
+   if (saved_result_storage.file == BAD_FILE) {
+      this->result = fs_reg(this, ir->type);
+   } else {
+      this->result = saved_result_storage;
+   }
 
    switch (ir->operation) {
    case ir_unop_logic_not:
@@ -906,6 +915,9 @@ fs_visitor::visit(ir_expression *ir)
    case ir_unop_sign:
       temp = fs_reg(this, ir->type);
 
+      /* Unalias the destination.  (imagine a = sign(a)) */
+      this->result = fs_reg(this, ir->type);
+
       emit(BRW_OPCODE_MOV, this->result, fs_reg(0.0f));
 
       inst = emit(BRW_OPCODE_CMP, reg_null_f, op[0], fs_reg(0.0f));
@@ -1054,6 +1066,9 @@ fs_visitor::visit(ir_expression *ir)
       break;
 
    case ir_binop_min:
+      /* Unalias the destination */
+      this->result = fs_reg(this, ir->type);
+
       inst = emit(BRW_OPCODE_CMP, this->result, op[0], op[1]);
       inst->conditional_mod = BRW_CONDITIONAL_L;
 
@@ -1061,6 +1076,9 @@ fs_visitor::visit(ir_expression *ir)
       inst->predicated = true;
       break;
    case ir_binop_max:
+      /* Unalias the destination */
+      this->result = fs_reg(this, ir->type);
+
       inst = emit(BRW_OPCODE_CMP, this->result, op[0], op[1]);
       inst->conditional_mod = BRW_CONDITIONAL_G;
 
@@ -1106,8 +1124,10 @@ fs_visitor::emit_assignment_writes(fs_reg &l, fs_reg &r,
         l.type = brw_type_for_base_type(type);
         r.type = brw_type_for_base_type(type);
 
-        fs_inst *inst = emit(BRW_OPCODE_MOV, l, r);
-        inst->predicated = predicated;
+        if (predicated || !l.equals(&r)) {
+           fs_inst *inst = emit(BRW_OPCODE_MOV, l, r);
+           inst->predicated = predicated;
+        }
 
         l.reg_offset++;
         r.reg_offset++;
@@ -1142,9 +1162,21 @@ fs_visitor::visit(ir_assignment *ir)
    fs_inst *inst;
 
    /* FINISHME: arrays on the lhs */
+   this->result = reg_undef;
    ir->lhs->accept(this);
    l = this->result;
 
+   /* If we're doing a direct assignment, an RHS expression could
+    * drop its result right into our destination.  Otherwise, tell it
+    * not to.
+    */
+   if (ir->condition ||
+       !(ir->lhs->type->is_scalar() ||
+        (ir->lhs->type->is_vector() &&
+         ir->write_mask == (1 << ir->lhs->type->vector_elements) - 1))) {
+      this->result = reg_undef;
+   }
+
    ir->rhs->accept(this);
    r = this->result;
 
@@ -1159,9 +1191,13 @@ fs_visitor::visit(ir_assignment *ir)
        ir->lhs->type->is_vector()) {
       for (int i = 0; i < ir->lhs->type->vector_elements; i++) {
         if (ir->write_mask & (1 << i)) {
-           inst = emit(BRW_OPCODE_MOV, l, r);
-           if (ir->condition)
+           if (ir->condition) {
+              inst = emit(BRW_OPCODE_MOV, l, r);
               inst->predicated = true;
+           } else if (!l.equals(&r)) {
+              inst = emit(BRW_OPCODE_MOV, l, r);
+           }
+
            r.reg_offset++;
         }
         l.reg_offset++;
@@ -1202,16 +1238,19 @@ fs_visitor::emit_texture_gen4(ir_texture *ir, fs_reg dst, fs_reg coordinate,
         emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), fs_reg(0.0f));
         mlen++;
       } else if (ir->op == ir_txb) {
+        this->result = reg_undef;
         ir->lod_info.bias->accept(this);
         emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), this->result);
         mlen++;
       } else {
         assert(ir->op == ir_txl);
+        this->result = reg_undef;
         ir->lod_info.lod->accept(this);
         emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), this->result);
         mlen++;
       }
 
+      this->result = reg_undef;
       ir->shadow_comparitor->accept(this);
       emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), this->result);
       mlen++;
@@ -1246,10 +1285,12 @@ fs_visitor::emit_texture_gen4(ir_texture *ir, fs_reg dst, fs_reg coordinate,
       mlen += 6;
 
       if (ir->op == ir_txb) {
+        this->result = reg_undef;
         ir->lod_info.bias->accept(this);
         emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), this->result);
         mlen++;
       } else {
+        this->result = reg_undef;
         ir->lod_info.lod->accept(this);
         emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), this->result);
         mlen++;
@@ -1341,6 +1382,7 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
    if (ir->shadow_comparitor) {
       mlen = MAX2(mlen, header_present + 4 * reg_width);
 
+      this->result = reg_undef;
       ir->shadow_comparitor->accept(this);
       emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), this->result);
       mlen += reg_width;
@@ -1352,6 +1394,7 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
       inst = emit(FS_OPCODE_TEX, dst);
       break;
    case ir_txb:
+      this->result = reg_undef;
       ir->lod_info.bias->accept(this);
       mlen = MAX2(mlen, header_present + 4 * reg_width);
       emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), this->result);
@@ -1361,6 +1404,7 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
 
       break;
    case ir_txl:
+      this->result = reg_undef;
       ir->lod_info.lod->accept(this);
       mlen = MAX2(mlen, header_present + 4 * reg_width);
       emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), this->result);
@@ -1464,6 +1508,7 @@ fs_visitor::visit(ir_texture *ir)
    int sampler;
    fs_inst *inst = NULL;
 
+   this->result = reg_undef;
    ir->coordinate->accept(this);
    fs_reg coordinate = this->result;
 
@@ -1609,6 +1654,7 @@ fs_visitor::visit(ir_texture *ir)
 void
 fs_visitor::visit(ir_swizzle *ir)
 {
+   this->result = reg_undef;
    ir->val->accept(this);
    fs_reg val = this->result;
 
@@ -1671,6 +1717,7 @@ fs_visitor::visit(ir_constant *ir)
       const unsigned size = type_size(ir->type->fields.array);
 
       for (unsigned i = 0; i < ir->type->length; i++) {
+        this->result = reg_undef;
         ir->array_elements[i]->accept(this);
         fs_reg src_reg = this->result;
 
@@ -1686,6 +1733,7 @@ fs_visitor::visit(ir_constant *ir)
         ir_instruction *const field = (ir_instruction *) node;
         const unsigned size = type_size(field->type);
 
+        this->result = reg_undef;
         field->accept(this);
         fs_reg src_reg = this->result;
 
@@ -1736,6 +1784,7 @@ fs_visitor::emit_bool_to_cond_code(ir_rvalue *ir)
       for (unsigned int i = 0; i < expr->get_num_operands(); i++) {
         assert(expr->operands[i]->type->is_scalar());
 
+        this->result = reg_undef;
         expr->operands[i]->accept(this);
         op[i] = this->result;
       }
@@ -1800,6 +1849,7 @@ fs_visitor::emit_bool_to_cond_code(ir_rvalue *ir)
       return;
    }
 
+   this->result = reg_undef;
    ir->accept(this);
 
    if (intel->gen >= 6) {
@@ -1829,6 +1879,7 @@ fs_visitor::emit_if_gen6(ir_if *ir)
       for (unsigned int i = 0; i < expr->get_num_operands(); i++) {
         assert(expr->operands[i]->type->is_scalar());
 
+        this->result = reg_undef;
         expr->operands[i]->accept(this);
         op[i] = this->result;
       }
@@ -1890,6 +1941,7 @@ fs_visitor::emit_if_gen6(ir_if *ir)
       return;
    }
 
+   this->result = reg_undef;
    ir->condition->accept(this);
 
    fs_inst *inst = emit(BRW_OPCODE_IF, reg_null_d, this->result, fs_reg(0));
@@ -1922,7 +1974,7 @@ fs_visitor::visit(ir_if *ir)
    foreach_iter(exec_list_iterator, iter, ir->then_instructions) {
       ir_instruction *ir = (ir_instruction *)iter.get();
       this->base_ir = ir;
-
+      this->result = reg_undef;
       ir->accept(this);
    }
 
@@ -1932,7 +1984,7 @@ fs_visitor::visit(ir_if *ir)
       foreach_iter(exec_list_iterator, iter, ir->else_instructions) {
         ir_instruction *ir = (ir_instruction *)iter.get();
         this->base_ir = ir;
-
+        this->result = reg_undef;
         ir->accept(this);
       }
    }
@@ -1955,10 +2007,14 @@ fs_visitor::visit(ir_loop *ir)
       counter = *(variable_storage(ir->counter));
 
       if (ir->from) {
+        this->result = counter;
+
         this->base_ir = ir->from;
+        this->result = counter;
         ir->from->accept(this);
 
-        emit(BRW_OPCODE_MOV, counter, this->result);
+        if (!this->result.equals(&counter))
+           emit(BRW_OPCODE_MOV, counter, this->result);
       }
    }
 
@@ -1966,6 +2022,7 @@ fs_visitor::visit(ir_loop *ir)
 
    if (ir->to) {
       this->base_ir = ir->to;
+      this->result = reg_undef;
       ir->to->accept(this);
 
       fs_inst *inst = emit(BRW_OPCODE_CMP, reg_null_cmp, counter, this->result);
@@ -1979,11 +2036,13 @@ fs_visitor::visit(ir_loop *ir)
       ir_instruction *ir = (ir_instruction *)iter.get();
 
       this->base_ir = ir;
+      this->result = reg_undef;
       ir->accept(this);
    }
 
    if (ir->increment) {
       this->base_ir = ir->increment;
+      this->result = reg_undef;
       ir->increment->accept(this);
       emit(BRW_OPCODE_ADD, counter, counter, this->result);
    }
@@ -2033,7 +2092,7 @@ fs_visitor::visit(ir_function *ir)
       foreach_iter(exec_list_iterator, iter, sig->body) {
         ir_instruction *ir = (ir_instruction *)iter.get();
         this->base_ir = ir;
-
+        this->result = reg_undef;
         ir->accept(this);
       }
    }
@@ -4117,6 +4176,7 @@ fs_visitor::run()
       foreach_iter(exec_list_iterator, iter, *shader->ir) {
         ir_instruction *ir = (ir_instruction *)iter.get();
         base_ir = ir;
+        this->result = reg_undef;
         ir->accept(this);
       }
 

diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
index 4b355c979eba9d7bad34dba583f3c6513e5f80f3..41e5004f37bd3d5d1da163c2dad994607c192f9a 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -571,7 +571,10 @@ public:
 
    bool failed;
 
-   /* Result of last visit() method. */
+   /* On entry to a visit() method, this is the storage for the
+    * result.  On exit, the visit() called may have changed it, in
+    * which case the parent must use the new storage instead.
+    */
    fs_reg result;
 
    fs_reg pixel_x;