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

[Francisco Jerez]

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.

> 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: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170109/a8f567f7/attachment.sig>