Re: [PATCH 1/2] Reorder APX insns in i386.tbl

[Jan Beulich <jbeulich@suse.com>]

On 15.11.2023 02:49, Hu, Lin1 wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, November 14, 2023 7:21 PM
>> To: Hu, Lin1 <lin1.hu@intel.com>
>> Cc: Lu, Hongjiu <hongjiu.lu@intel.com>; binutils@sourceware.org
>> Subject: Re: [PATCH 1/2] Reorder APX insns in i386.tbl
>>
>> On 14.11.2023 03:58, Hu, Lin1 wrote:
>>> ---
>>>  gas/config/tc-i386.c |     4 +-
>>>  opcodes/i386-opc.tbl |   156 +-
>>>  5 files changed, 13189 insertions(+), 10771 deletions(-)
>>
>> What was the goal of sending this patch to the list, without any further
>> comments or explanations? It quite clearly doesn't apply to the present code
>> base.
>>
>> The diffstat is pretty odd, too: There clearly aren't as many files/lines changed.
> 
> This patch is just to show you the changes I would need to make if I reorder .tbl, the big difference in lines is because I removed the changes to mnem.h, init.h, etc. in the email.

Okay, but this isn't going to be a separate patch. It ought to be merged
into the patch introducing the new templates, so there's not going to be
extra diff or churn.

>>> @@ -2124,12 +2126,12 @@ xcryptofb, 0xf30fa7e8, PadLock,
>>> NoSuf|RepPrefixOk, {}  xstore, 0xfa7c0, PadLock, NoSuf|RepPrefixOk, {}
>>>
>>>  // Multy-precision Add Carry, rdseed instructions.
>>> +adcx, 0x6666, ADX|APX_F,
>>>
>> +C|Modrm|CheckOperandSize|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|VexVVV
>> VDe
>>> +st|EVex128|EVexMap4, { Reg32|Reg64|Unspecified|BaseIndex,
>>> +Reg32|Reg64, Reg32|Reg64 }
>>>  adcx, 0x660f38f6, ADX,
>>> Modrm|CheckOperandSize|IgnoreSize|No_bSuf|No_wSuf|No_sSuf, {
>>> Reg32|Reg64|Unspecified|BaseIndex, Reg32|Reg64 }  adcx, 0x6666,
>>> ADX|APX_F,
>>>
>> Modrm|CheckOperandSize|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|EVex128|E
>> Vex
>>> Map4, { Reg32|Reg64|Unspecified|BaseIndex, Reg32|Reg64 } -adcx,
>>> 0x6666, ADX|APX_F,
>>>
>> Modrm|CheckOperandSize|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|VexVVVVD
>> est|
>>> EVex128|EVexMap4, { Reg32|Reg64|Unspecified|BaseIndex, Reg32|Reg64,
>>> Reg32|Reg64 }
>>> +adox, 0xf366, ADX|APX_F,
>>>
>> +C|Modrm|CheckOperandSize|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|VexVVV
>> VDe
>>> +st|EVex128|EVexMap4, { Reg32|Reg64|Unspecified|BaseIndex,
>>> +Reg32|Reg64, Reg32|Reg64 }
>>>  adox, 0xf30f38f6, ADX,
>>> Modrm|CheckOperandSize|IgnoreSize|No_bSuf|No_wSuf|No_sSuf, {
>>> Reg32|Reg64|Unspecified|BaseIndex, Reg32|Reg64 }  adox, 0xf366,
>>> ADX|APX_F,
>>>
>> Modrm|CheckOperandSize|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|EVex128|E
>> Vex
>>> Map4, { Reg32|Reg64|Unspecified|BaseIndex, Reg32|Reg64 } -adox,
>>> 0xf366, ADX|APX_F,
>>>
>> Modrm|CheckOperandSize|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|VexVVVVD
>> est|
>>> EVex128|EVexMap4, { Reg32|Reg64|Unspecified|BaseIndex, Reg32|Reg64,
>>> Reg32|Reg64 }
>>
>> What is this part about? I thought we agreed that ADCX/ADOX aren't suitable for
>> NDD->REX2 optimization, at which point the ordering of templates here could as
>> well be left alone.
>>
>       62 54 bd 18 66 c7       adcx   %r15,%r8,%r8
>       66 4d 0f  38 f6  c7       adcx   %r15,%r8
>       
> The code can optimize adcx/adox from NDD to legacy, currently. Do you mean we don't consider the optimization If the code length remain the same?

As said before, any optimization we do has to actually result in some sort
of win. If neither performance nor code size change, there's no point in
making any adjustments to what the user has written.

That said, though: Looks like there would still be a 1-byte win for 32-bit
ADCX/ADOX with just the low 8 registers used as operands, i.e. when no REX
prefix is needed in the encoding.

Jan