[PATCH] x86: Use target("general-regs-only,baseline-isas-only") in <cpuid.h>

[Uros Bizjak]

On Tue, Aug 25, 2020 at 2:13 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Aug 24, 2020 at 12:40 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Mon, Aug 24, 2020 at 12:25 PM Uros Bizjak <ubizjak@gmail.com> wrote:
> > >
> > > On Mon, Aug 24, 2020 at 6:17 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > On Mon, Aug 24, 2020 at 7:55 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> > > > >
> > > > > On Mon, Aug 24, 2020 at 3:23 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > >
> > > > > > > Speaking of pragmas, these should be added outside cpuid.h, like:
> > > > > > >
> > > > > > > #pragma GCC push_options
> > > > > > > #pragma GCC target("general-regs-only")
> > > > > > >
> > > > > > > #include <cpuid.h>
> > > > > > >
> > > > > > > void cpuid_check ()
> > > > > > > ...
> > > > > > >
> > > > > > > #pragma GCC pop_options
> > > > > > >
> > > > > > > >footnote
> > > > > > >
> > > > > > > Nowadays, -march=native is mostly used outside generic target
> > > > > > > compilations, so for relevant avx512 targets, we still generate spills
> > > > > > > to mask regs. In future, we can review the setting of the tuning flag
> > > > > > > for a generic target in the same way as with SSE2 inter-reg moves.
> > > > > > >
> > > > > >
> > > > > > Florian raised an issue that we need to limit <cpuid.h> to the basic ISAs.
> > > > > > <cpuid.h> should be handled similarly to other intrinsic header files.
> > > > > > That is <cpuid.h> should use
> > > > > >
> > > > > > #pragma GCC push_options
> > > > > > #ifdef __x86_64__
> > > > > > #pragma GCC target("arch=x86-64")
> > > > > > #else
> > > > > > #pragma GCC target("arch=i386")
> > > > > > ...
> > > > > > #pragma GCC pop_options
> > > > > >
> > > > > > Here is a patch.  OK for master?
> > > > >
> > > > > -ENOPATCH
> > > > >
> > > > > However, how will this affect inlining? Every single function in
> > > > > cpuid.h is defined as static __inline, and due to target flags
> > > > > mismatch, it won't be inlined anymore. These inline functions are used
> > > > > in some bit testing functions, and to keep them inlined, these should
> > > > > also use the same options to avoid non-basic ISAs. This is the reason
> > > > > cpuid.h should be #included after pragma, together with bit testing
> > > > > functions, as shown above.
> > > > >
> > > >
> > > > How about target("baseline-isas-only")? All CPUID functions are
> > > > inlined.
> > >
> > > No, I don't think this is a good idea. Now consider the situation that
> > > caller functions are compiled with e.g. -mgeneral-regs-only. Due to
> > > #pragmas, CPUID functions are compiled with a superset ISAs, so they
> > > again won't be inlined. ISAs of caller functions and CPUID should
> > > match, the best way is to include <cpuid.h> after the #pragma. And
> > > IMO, general-regs-only target #pragma is an excellent setting for
> > > both: cpuid.h and caller bit testing functions.
> > >
> > > So, if we care about inlining, decorating cpuid.h with target pragmas
> > > is a bad idea.
> >
> > This can be done with #pragma in <cpuid.h>.
> >
>
> We just need to update ix86_can_inline_p to allow inline functions
> with baseline-isas-only and general-regs-only attributes if caller
> supports the same set of ISAs.
>
> Here is the updated patch.

I'm not against it, but I don't plan to approve the attached patch.

Uros.