[Mesa-dev] [PATCH 43/95] i965/disasm: print NibCtrl for instructions with execsize 4

[Iago Toral]

On Mon, 2016-08-08 at 15:58 -0700, Francisco Jerez wrote:
> Iago Toral Quiroga <itoral at igalia.com> writes:
> 
> > 
> > ---
> >  src/mesa/drivers/dri/i965/brw_disasm.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c
> > b/src/mesa/drivers/dri/i965/brw_disasm.c
> > index c8bdeab..d5e9916 100644
> > --- a/src/mesa/drivers/dri/i965/brw_disasm.c
> > +++ b/src/mesa/drivers/dri/i965/brw_disasm.c
> > @@ -1169,7 +1169,13 @@ qtr_ctrl(FILE *file, const struct
> > brw_device_info *devinfo, brw_inst *inst)
> >     int qtr_ctl = brw_inst_qtr_control(devinfo, inst);
> >     int exec_size = 1 << brw_inst_exec_size(devinfo, inst);
> >  
> > -   if (exec_size == 8) {
> > +   if (exec_size == 4) {
> I guess it wouldn't hurt to show this for exec_size < 4 too even
> though
> it will typically be 1N.

The PRMs say:

"NibCtrl is only allowed for SIMD4 instructions with a DF (Double
Float) source or destination type"

I don't know if the "only allowed" in that sentence means that it is
incorrect to set it to to anything else in instructions with an exec
size < 4, in which case showing it in the disassembly would help find
bugs although maybe we should just validate for that when we generate
code, or if it means that the hardware will ignore it completely, in
which case showing its value does not seem useful for anything.

What do you think?

> > 
> > +      int nib_ctl = brw_inst_nib_control(devinfo, inst);
> This may cause an assertion failure on SNB and earlier because the
> NibCtrl field doesn't exist.  You could do something along the lines
> of:
> 
> > 
> > const unsigned nib_ctl = devinfo->gen < 7 ? 0 :
> >                          brw_inst_nib_control(devinfo, inst);

Ah, good to know, thanks!

> > +      if (nib_ctl == 0)
> > +         string(file, " 1N");
> > +      else
> > +         string(file, " 2N");
> The usual qtr_ctl field is still taken into account by the hardware
> regardless of whether you use NibCtrl, this should probably be:
> 
> > 
> > format(file, " %dN", qtr_ctl * 2 + nib_ctl + 1);

Yeah, good point.

> > +   } else if (exec_size == 8) {
> >        switch (qtr_ctl) {
> >        case 0:
> >           string(file, " 1Q");