i965: Fix negate with unsigned integers

For code such as:

uint tmp1 = uint(in0);
uint tmp2 = -tmp1;
float out0 = float(tmp2);

We produce code like:
mov(8)    g5<1>.xF    -g9<4,4,1>.xUD

which does not produce correct results. This code produces the
results we would expect if tmp1 and tmp2 were signed integers
instead.

It seems that a similar problem was detected and addressed when
using negations with unsigned integers as part of condionals, but
it looks like the problem has a wider impact than that.

This patch fixes the problem by preventing copy-propagation of
negated UD registers in all scenarios, not only in conditionals.

Fixes the following 24 dEQP tests:

dEQP-GLES3.functional.shaders.operator.unary_operator.minus.*_uint_*
dEQP-GLES3.functional.shaders.operator.unary_operator.minus.*_uvec2_*
dEQP-GLES3.functional.shaders.operator.unary_operator.minus.*_uvec3_*
dEQP-GLES3.functional.shaders.operator.unary_operator.minus.*_uvec4_*

Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>

diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
index 70f417fd2a8bac3a5837e8bcd2a173c6415e69af..5dd7255929879c59f0b985a1242b8be46bbe9326 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
@@ -302,9 +302,12 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
        (entry->dst.reg_offset + entry->regs_written) * 32)
       return false;
 
-   /* See resolve_ud_negate() and comment in brw_fs_emit.cpp. */
-   if (inst->conditional_mod &&
-       inst->src[arg].type == BRW_REGISTER_TYPE_UD &&
+   /* we can't generally copy-propagate UD negations because we
+    * can end up accessing the resulting values as signed integers
+    * instead. See also resolve_ud_negate() and comment in
+    * fs_generator::generate_code.
+    */
+   if (inst->src[arg].type == BRW_REGISTER_TYPE_UD &&
        entry->src.negate)
       return false;
 

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
index bf15941e10214beaeb28496d567e72670ccdec90..e25c9953c1c9de05e7165cae0e62efbe7059cae2 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
@@ -314,12 +314,11 @@ try_copy_propagate(struct brw_context *brw, vec4_instruction *inst,
    if (inst->is_send_from_grf())
       return false;
 
-   /* We can't copy-propagate a UD negation into a condmod
-    * instruction, because the condmod ends up looking at the 33-bit
-    * signed accumulator value instead of the 32-bit value we wanted
+   /* we can't generally copy-propagate UD negations becuse we
+    * end up accessing the resulting values as signed integers
+    * instead. See also resolve_ud_negate().
     */
-   if (inst->conditional_mod &&
-       value.negate &&
+   if (value.negate &&
        value.type == BRW_REGISTER_TYPE_UD)
       return false;