From 6dd346c2328df86f843c3a355a7919fb0404f6df Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Mon, 19 Jan 2015 12:32:09 +0100 Subject: [PATCH] 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 --- src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 9 ++++++--- src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp | 9 ++++----- 2 files changed, 10 insertions(+), 8 deletions(-) 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 70f417f..5dd7255 100644 --- 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 bf15941..e25c995 100644 --- 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; -- 2.7.4