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

[Matt Turner]

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.

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}.