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

[Cui, Lili]

> > > > -----Original Message-----
> > > > From: H.J. Lu <hjl.tools@gmail.com>
> > > > Sent: Monday, September 16, 2024 9:26 PM
> > > > To: Beulich, Jan <JBeulich@suse.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 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.  Compilers always
> > > > generate valid APX instructions.  Assembly codes with EGPR should also
> follow APX spec.
> > > > These bogus EGPR instructions don't really help EGPR usage.
> > > >
> > >
> > > For this type of optimization, we discussed this issue very carefully in the
> internal meeting last time, including different compilers, which is also the source
> of the opinions listed in my first post. For currently illegal usage, everyone
> recommends reporting errors.  I also specifically asked the authors of APX spec,
> and they also think that errors should be reported.
> > >
> >
> > I will check in my patch today.
> >
> 
> I am backporting it to 2.43 branch.
> 

This is indeed necessary.

Thanks,
Lili.

> --
> H.J.