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

[H.J. Lu]

On Wed, Sep 18, 2024 at 9:44 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Sep 18, 2024 at 9:15 AM Cui, Lili <lili.cui@intel.com> wrote:
> >
> >
> >
> > > -----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.

-- 
H.J.