i965/fs: Fix comparisions with uint negation.

The condmod instruction ends up generating garbage condition codes,
because apparently the comparison happens on the accumulator value (33
bits for UD), not the truncated value that would be written.

Fixes fs-op-neg-*

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 738610621b1ee21fb0a36a617ebc7e61b94e6f0d..c0d93c03a2e7a39e17306d644b95ca6072c39aad 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -1347,6 +1347,19 @@ fs_visitor::register_coalesce()
            interfered = true;
            break;
         }
+
+        /* The accumulator result appears to get used for the
+         * conditional modifier generation.  When negating a UD
+         * value, there is a 33rd bit generated for the sign in the
+         * accumulator value, so now you can't check, for example,
+         * equality with a 32-bit value.  See piglit fs-op-neg-uint.
+         */
+        if (scan_inst->conditional_mod &&
+            inst->src[0].negate &&
+            inst->src[0].type == BRW_REGISTER_TYPE_UD) {
+           interfered = true;
+           break;
+        }
       }
       if (interfered) {
         continue;

diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
index f344330db1dace3c275da5e7d840190167672528..f5af9ea936a38ae1296b350f48fc4198955f51d1 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -535,6 +535,7 @@ public:
                               fs_inst *last_rhs_inst);
    void emit_assignment_writes(fs_reg &l, fs_reg &r,
                               const glsl_type *type, bool predicated);
+   void resolve_ud_negate(fs_reg *reg);
 
    struct brw_reg interp_reg(int location, int channel);
    int setup_uniform_values(int loc, const glsl_type *type);

diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
index 88a999965a1b1d18c1bb6c02b06d9fbf1cbb4dad..0c14baf367d0c791e42910150d55419f32c7f864 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
@@ -635,6 +635,16 @@ fs_visitor::generate_code()
 
       for (unsigned int i = 0; i < 3; i++) {
         src[i] = brw_reg_from_fs_reg(&inst->src[i]);
+
+        /* The accumulator result appears to get used for the
+         * conditional modifier generation.  When negating a UD
+         * value, there is a 33rd bit generated for the sign in the
+         * accumulator value, so now you can't check, for example,
+         * equality with a 32-bit value.  See piglit fs-op-neg-uvec4.
+         */
+        assert(!inst->conditional_mod ||
+               inst->src[i].type != BRW_REGISTER_TYPE_UD ||
+               !inst->src[i].negate);
       }
       dst = brw_reg_from_fs_reg(&inst->dst);
 

diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index e523ae748bcf5a57e36b8ab26ac4df813f2d5395..215cdcdb02200b17ea59d8a08e90f6f60e787572 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -317,6 +317,9 @@ fs_visitor::visit(ir_expression *ir)
       if (intel->gen < 5)
         temp.type = op[0].type;
 
+      resolve_ud_negate(&op[0]);
+      resolve_ud_negate(&op[1]);
+
       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));
@@ -377,6 +380,8 @@ fs_visitor::visit(ir_expression *ir)
       if (intel->gen < 5)
         temp.type = op[0].type;
 
+      resolve_ud_negate(&op[0]);
+
       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));
@@ -401,6 +406,9 @@ fs_visitor::visit(ir_expression *ir)
       break;
 
    case ir_binop_min:
+      resolve_ud_negate(&op[0]);
+      resolve_ud_negate(&op[1]);
+
       if (intel->gen >= 6) {
         inst = emit(BRW_OPCODE_SEL, this->result, op[0], op[1]);
         inst->conditional_mod = BRW_CONDITIONAL_L;
@@ -416,6 +424,9 @@ fs_visitor::visit(ir_expression *ir)
       }
       break;
    case ir_binop_max:
+      resolve_ud_negate(&op[0]);
+      resolve_ud_negate(&op[1]);
+
       if (intel->gen >= 6) {
         inst = emit(BRW_OPCODE_SEL, this->result, op[0], op[1]);
         inst->conditional_mod = BRW_CONDITIONAL_GE;
@@ -1369,6 +1380,8 @@ fs_visitor::emit_bool_to_cond_code(ir_rvalue *ir)
 
         expr->operands[i]->accept(this);
         op[i] = this->result;
+
+        resolve_ud_negate(&op[i]);
       }
 
       switch (expr->operation) {
@@ -1984,3 +1997,15 @@ fs_visitor::emit_fb_writes()
 
    this->current_annotation = NULL;
 }
+
+void
+fs_visitor::resolve_ud_negate(fs_reg *reg)
+{
+   if (reg->type != BRW_REGISTER_TYPE_UD ||
+       !reg->negate)
+      return;
+
+   fs_reg temp = fs_reg(this, glsl_type::uint_type);
+   emit(BRW_OPCODE_MOV, temp, *reg);
+   *reg = temp;
+}