[Mesa-dev] [PATCH 01/22] i965/disasm: also print nibctrl in IVB for execsize=8

[Samuel Iglesias Gonsálvez]

On Mon, 2017-01-09 at 14:42 -0800, Francisco Jerez wrote:
> Matt Turner <mattst88 at gmail.com> writes:
> 
> > On 01/05, Samuel Iglesias Gonsálvez wrote:
> > > From: Iago Toral Quiroga <itoral at igalia.com>
> > > 
> > > 4-wide DF operations where NibCtrl applies require and execsize
> > > of 8
> > > in IvyBridge/Valleyview.
> > 
> > Wow, the documentation is bad in this area. The QtrCtrl description
> > in
> > IVB's Vol4 Part3 explicitly says "NibCtrl is only allowed for SIMD4
> > instructions with (DF) double precision source and/or destination."
> > and
> > shows NibCtrl only being enabled with ExecSize=4. That's very
> > unclear
> > and I hope that didn't trip you guys up.
> > 
> > > ---
> > > src/mesa/drivers/dri/i965/brw_disasm.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c
> > > b/src/mesa/drivers/dri/i965/brw_disasm.c
> > > index 167067a..7c3791d 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_disasm.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_disasm.c
> > > @@ -1209,6 +1209,13 @@ qtr_ctrl(FILE *file, const struct
> > > gen_device_info *devinfo, brw_inst *inst)
> > >          string(file, " 4Q");
> > >          break;
> > >       }
> > > +      if (devinfo->gen == 7 && !devinfo->is_haswell) {
> > > +         int nib_ctl = brw_inst_nib_control(devinfo, inst);
> > > +         if (nib_ctl == 0)
> > > +            string(file, " 1N");
> > > +         else
> > > +            string(file, " 2N");
> > > +      }
> > 
> > This looks like it will print 1N/2N in addition to the 1Q/2Q/3Q/4Q
> > from
> > immediately before. I don't think that's the intention.
> > 
> > Perhaps we can do a slight refactor and change the structure to 
> > 
> > 	const unsigned nib_ctrl = devinfo->gen < 7 ? 0 :
> > 				  brw_inst_nib_control(devinfo, inst);
> > 	if (nib_ctrl) {
> > 		format(file, " %dN", qtr_ctl * 2 + nib_ctl + 1);
> > 	} else if (exec_size == 8) {
> > 		...
> > 	} else if (exec_size == 16) {
> > 		...
> > 	}
> > 
> 
> There's code already to print the nib ctrl field a few lines
> above.  You
> just need to enable it for the nib_ctrl != 0 case in addition to
> exec_size being less than 8.
> 

Right, much simpler. I will do this change.

Thanks,

Sam

> > this is just the disassembler, so we don't need to implement logic
> > that
> > decides when NibCtrl is valid and when it's not -- just disassemble
> > what's there :)
> > 
> > (I renamed nib_ctl -> nib_ctrl in the above block intentionally to
> > match
> > the field name).
> > 
> > You can preemptively put a
> > 
> > Reviewed-by: Matt Turner <mattst88 at gmail.com>
> > 
> > on such a patch.
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170110/b0e493f8/attachment.sig>