On Mon, Jan 19, 2015 at 3:32 AM, Eduardo Lima Mitev <el...@igalia.com> wrote: > 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
Changes look good to me. Verified that patch now generates correct code for the example in the comment. Reviewed-by: Anuj Phogat <anuj.pho...@gmail.com> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
