[Mesa-dev] [PATCH] i965/vec4: Use UW type for multiply into accumulator on GEN8+

[Jason Ekstrand]

On Tue, Jan 19, 2016 at 12:57 PM, Matt Turner <mattst88 at gmail.com> wrote:

> On Sat, Jan 16, 2016 at 7:31 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> > On Jan 16, 2016 5:56 PM, "Matt Turner" <mattst88 at gmail.com> wrote:
> >>
> >> On Thu, Jan 14, 2016 at 12:27 PM, Matt Turner <mattst88 at gmail.com>
> wrote:
> >> > On Thu, Jan 14, 2016 at 12:08 PM, Jason Ekstrand <
> jason at jlekstrand.net>
> >> > wrote:
> >> >> BDW adds the following restriction: "When multiplying DW x DW, the
> dst
> >> >> cannot be accumulator."
> >> >> ---
> >> >>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 6 +++++-
> >> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> >> >> b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> >> >> index c228743..b2335bd 100644
> >> >> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> >> >> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> >> >> @@ -1069,7 +1069,11 @@ vec4_visitor::nir_emit_alu(nir_alu_instr
> *instr)
> >> >>     case nir_op_umul_high: {
> >> >>        struct brw_reg acc = retype(brw_acc_reg(8), dst.type);
> >> >>
> >> >> -      emit(MUL(acc, op[0], op[1]));
> >> >> +      if (devinfo->gen >=8)
> >> >
> >> > Space after >=
> >> >
> >> >> +         emit(MUL(acc, op[0], retype(op[1], BRW_REGISTER_TYPE_UW)));
> >> >> +      else
> >> >> +         emit(MUL(acc, op[0], op[1]));
> >> >> +
> >> >
> >> > Do the
> >> >
> tests/spec/arb_gpu_shader5/execution/built-in-functions/vs-{i,u}mulExtended*.shader_test
> >> > tests currently fail on BDW with INTEL_DEBUG=vec4? If so, presumably
> >> > this fixes it?
> >
> > It didn't fix anything
>
> That's an aggravatingly ambiguous answer. Did it not fix anything
> because all the tests already passed, or did it not fix anything
> because the tests still fail?
>
> I pulled out my Broadwell to test for myself:
>
> First column is 0a68112~, second is 0a68112~ using INTEL_DEBUG=vec4,
> third is 0a68112 using INTEL_DEBUG=vec4.
>
> vs-imulextended:                     pass fail pass
> vs-imulextended-nonuniform:          pass fail pass
> vs-imulextended-only-msb:            pass fail pass
> vs-imulextended-only-msb-nonuniform: pass fail pass
> vs-umulextended:                     pass fail pass
> vs-umulextended-nonuniform:          pass fail pass
> vs-umulextended-only-msb:            pass fail pass
> vs-umulextended-only-msb-nonuniform: pass fail pass
>
> So the tests I asked you to run did fail without this patch and were
> fixed by this patch.
>

Ok, I see what's going on.  We have no GS variants of those tests on BDW so
of course Jenkins didn't give me a difference thanks to SIMD8 VS.   Sorry
for the confusion; I wasn't paying enough attention.
--Jason


>
> For completeness, after those tests were fixed by this patch, the only
> remaining regressions from INTEL_DEBUG=vec4 on BDW are
>
> spec/arb_shader_draw_parameters/drawid-{indirect-baseinstance,basevertex,vertexid}.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20160119/101f57a5/attachment-0001.html>