Re: [PATCH] x86: infer No_*Suf from other insn attributes

[Jan Beulich <jbeulich@suse.com>]

On 15.11.2022 00:33, H.J. Lu wrote:
> On Mon, Nov 14, 2022 at 8:12 AM Jan Beulich <jbeulich@suse.com> wrote:
>> --- a/opcodes/i386-opc.tbl
>> +++ b/opcodes/i386-opc.tbl
>> @@ -75,12 +75,17 @@
>>  #define Size32 Size=SIZE32
>>  #define Size64 Size=SIZE64
>>
>> +#define IsPrefix IsPrefix|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf
>> +
> 
> I prefer to add
> 
> #define No_Suf No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf
> 
> to cover more templates.

Iirc you said so two years ago already in the context of "x86: imply
all No_*Suf when none is set in a template". Yet as before I don't
like going that route, as that still leaves clutter on the respective
lines (even if it's less clutter then). Plus the ultimate goal, as
also said back then, ought to be to move from negative to positive
forms. Doing things the way done here will avoid touching all those
lines again which are being touched here.

As a compromise I'd accept introducing NoSuf (or No_Suf) in addition
to the changes done here, for use on applicable lines not touched
here already, and for use in the #define-s I'm adding. I'd prefer
this to be a separate, subsequent patch though (to limit patch size,
focusing on one transformation at a time. (I could introduce the new
macro in a prereq patch, using it for only AddrPrefixOpReg right away,
then have the patch here use it in the new macros, and finally add one
to use the new macro on the remaining applicable templates.)

>> @@ -125,6 +130,11 @@
>>  #define VecSIB512 SIB=VECSIB512
>>  #define Sibmem SIB=SIBMEM|Modrm
>>
>> +#define SIB        No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SIB
> 
> Where is this used?

Half of the uses are even visible in patch context of this very hunk.
The other two are immediately ahead, just outside of patch context.

One more general request, which is particularly relevant on a large
patch like this one: Can you please trim reply context? Below here
you did leave over a thousand lines of patch content, leaving me and
any potential other reader to scroll through to see whether there's
any further comment.

Thanks, Jan