From: Iago Toral Quiroga <ito...@igalia.com> 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 8 dEQP tests: dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uint_vertex dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uint_fragment dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uvec2_vertex dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uvec2_fragment dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uvec3_vertex dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uvec3_fragment dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uvec4_vertex dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uvec4_fragment Note: For some reason the mediump and lowp versions of these tests still fail but I am not sure about the reason for that since the code we generate now seems correct (in fact, is the same as for the highp versions). These tests would need further investigation. --- 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 9e47dd9..562ecb7 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp @@ -318,12 +318,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.1.3 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev