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

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Nov 17, 2022 at 12:50 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 17.11.2022 02:54, H.J. Lu wrote:
> > On Mon, Nov 14, 2022 at 11:27 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> 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.)
> >
> > An explicit NoSuf (or No_Suf) is better.
>
> But why? As said - once switching to expressing permitted suffixes in a
> "positive" way that'll lead to touching again _all_ the templates I'm
> touching here (and of course many more I'm not touching here). Plus
> that's still one more item on (often long) lines which don't really need
> it. Plus, just to restate what the description says, we're already doing
> this with AddrPrefixOpReg.

I checked in a patch to define NoSuf and add it to AddrPrefixOpReg
in templates.

> I'd really like to see a way forward here - this is now my 2nd attempt
> that you're in the process of rejecting. If you insist on going the
> NoSuf-everywhere route (and hence you're not willing to accept the
> suggested compromise), then may I please ask that you make such a patch
> (within a reasonable time frame), for me to then re-base over? I'm not
> going to submit patches which I see clear downsides with. Yet further
> work I have pending wants this sorted (at least to limit patch size
> some). To be specific, I'm meaning to eliminate the bogus
> LONG_DOUBLE_MNEM_SUFFIX and then also No_ldSuf, which prior to the patch
> here means touching about every insn template. Yet even beyond that
> the size of many patches I have pending benefits from this size / line
> length reduction, just like the opcode table itself does.
>
> Jan



-- 
H.J.