On Saturday, June 11, 2016 4:06:32 PM PDT Jordan Justen wrote: > On 2016-06-10 14:19:44, Kenneth Graunke wrote: > > Cherryview and Broxton don't support DW x DW multiplication. We have > > piles of code to handle this, but apparently weren't retyping in the > > immediate case. > > > > For example, > > tests/spec/arb_tessellation_shader/execution/dvec3-vs-tcs-tes > > makes the simulator angry about instructions such as: > > > > mul(8) r18<1>:D r10.0<8;8,1>:D 0x00000003:D > > > > Just retype to UW. It should be safe everywhere. > > > > Cc: "12.0" <mesa-sta...@lists.freedesktop.org> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95462 > > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > > --- > > src/mesa/drivers/dri/i965/brw_fs.cpp | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp > > b/src/mesa/drivers/dri/i965/brw_fs.cpp > > index 4b29ee5..13246c2 100644 > > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp > > @@ -3564,7 +3564,10 @@ fs_visitor::lower_integer_multiplication() > > ibld.MOV(imm, inst->src[1]); > > ibld.MUL(inst->dst, imm, inst->src[0]); > > } else { > > - ibld.MUL(inst->dst, inst->src[0], inst->src[1]); > > + const bool ud = (inst->src[1].type == BRW_REGISTER_TYPE_UD); > > + ibld.MUL(inst->dst, inst->src[0], > > + ud ? brw_imm_uw(inst->src[1].ud) > > + : brw_imm_w(inst->src[1].d)); > > This change looks fine, but will it actually be possible to hit this > code path for negative numbers? Above, we have: > > if (inst->src[1].file == IMM && > inst->src[1].ud < (1 << 16)) { > > Bit 31 would be set if inst->src[1].d has a negative number. > > -Jordan
Good catch. I don't think it can be hit for negative numbers currently.
(Perhaps we should improve that check so we can handle those, someday?)
But we can still get positive numbers with D type that fit in 16 bits.
Here, I was just trying to preserve the signedness of the original type
so that we convert a D * D multiply into D * W rather than D * UW. It
seemed safer. Maybe it doesn't matter, though.
I've updated the last line of the commit message to:
Just retype to W or UW. It should be safe on all platforms.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
