[PATCH] x86/APX: Don't promote AVX512 instructions out of APX spec

[Cui, Lili]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, September 23, 2024 2:15 PM
> To: H.J. Lu <hjl.tools@gmail.com>
> Cc: Binutils <binutils@sourceware.org>; Cui, Lili <lili.cui@intel.com>
> Subject: Re: [PATCH] x86/APX: Don't promote AVX512 instructions out of APX
> spec
> 
> On 16.09.2024 15:25, H.J. Lu wrote:
> > On Mon, Sep 16, 2024 at 7:53 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 16.09.2024 08:46, H.J. Lu wrote:
> >>> On Mon, Sep 16, 2024 at 2:38 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 16.09.2024 08:32, H.J. Lu wrote:
> >>>>> On Mon, Sep 16, 2024 at 2:01 PM Jan Beulich <jbeulich@suse.com>
> wrote:
> >>>>>>
> >>>>>> On 16.09.2024 01:38, H.J. Lu wrote:
> >>>>>>> V{BROADCAST,EXTRACT,INSERT}{F,I}128 and VROUND{P,S}{S,D} aren't
> >>>>>>> promoted to support EGPR in APX spec.  Don't promote them out of
> APX spec.
> >>>>>>
> >>>>>> As previously indicated - I disagree with outright removal. If
> >>>>>> you want to have a strict mode, which compilers may want to enable,
> that's fine.
> >>>>>
> >>>>> There is no need to allow them.
> >>>>
> >>>> "Them" being what in this context? And regardless of the answer,
> >>>> this
> >>>
> >>> "them" are these invalid EGPR instructions.
> >>>
> >>>> likely will end up being your personal view, which others may or
> >>>> may not agree with.
> >>>>
> >>>>>> For hand-written assembly (possibly including inline assembly,
> >>>>>> yet there
> >>>>>
> >>>>> Adding them doesn't solve the inline assembly issue.  By default,
> >>>>> GCC doesn't allow EGPR in asm statements.  This option is needed
> >>>>> to enable EGPR in asm statements:
> >>>>>
> >>>>> ‘-mapx-inline-asm-use-gpr32’
> >>>>>      For inline asm support with APX, by default the EGPR feature was
> >>>>>      disabled to prevent potential illegal instruction with EGPR occurs.
> >>>>>      To invoke egpr usage in inline asm, use new compiler option
> >>>>>      -mapx-inline-asm-use-gpr32 and user should ensure the instruction
> >>>>>      supports EGPR.
> >>>>
> >>>> And with that option in use, said insns may end up failing to
> >>>> assemble. In a manner one may call random, as it'll depend on the
> >>>> compiler's register selection.
> >>>
> >>> That is why there are "user should ensure the instruction supports EGPR."
> >>>
> >>>>>> are sharp edges there) we ought to aid programmers. We aren't
> >>>>>> obliged to follow the spec to the letter, especially when the
> >>>>>> spec itself can be
> >>>>>
> >>>>> A spec is a spec even though you don't agreed with it.  We should
> >>>>> follow the APX spec.
> >>>>
> >>>> We need to do all the spec says, yes. The spec doesn't prevent us
> >>>> from doing more though, as long as it's compatible / functionally
> >>>> correct.
> >>>
> >>> Re-encode APX instructions with non-APX encoding is OK.
> >>
> >> This is fine as an optimization, sure.
> >>
> >>> But we shouldn't add invalid EGPR instructions per APX spec.
> >>
> >> You continue to say so without providing any good reason. What's
> >> wrong with going beyond the spec and helping the user at least a
> >> little to get past some of the oddities of the spec? As it stands I
> >> remain unconvinced of this step backwards, and hence the change
> >> shouldn't be put in. I'm not going to exclude though that with
> >> appropriate reasoning I might change my mind.
> >
> > Following APX spec is a very good reason.
> 
> You still put it as if I asked to not follow the spec. I'm instead arguing for going
> beyond the spec, without violating it in any way.
> 

Jan, I'm sorry that I didn't notice you checking in these patches at first, although I have been against this kind of promotion in the previous discussion. But it's really a bit radical to make this kind of out-of-spec promotion by default. Would you rather put them under options? After all, these conversions are very clever.

Thanks,
Lili.