Re: [PATCH v4 2/2] x86: Add general_regs_only function attribute

[Jakub Jelinek <jakub@redhat.com>]

On Wed, Apr 21, 2021 at 06:01:07PM -0700, H.J. Lu via Gcc-patches wrote:
> How about this?
> 
> @item general_regs_only
> @cindex @code{general_regs_only} function attribute, x86
> The @code{general_regs_only} function attribute informs the compiler
> that the function uses only general purpose registers.  When the
> compiler inlines a function with the @code{always_inline} attribute,
> target-specific compilation options may lead to inline failures.
> The @code{general_regs_only} attribute, if applicable, can be used
> together with the @code{always_inline} attribute to reduce inlining
> failure.

I don't really like this attribute.
It is very specific to what you want to solve and doesn't address the
general problem, that always_inline means different things in different
code, and that is a problem for many targets, not just one.

As has been written, in some cases it means inline always, error
whenever it is called indirectly which can't be optimized into a direct
call that can be inlined and error whenever the inlining fails for other
reasons.

Another case, e.g. in the glibc fortify wrappers, is inline always
when the call is direct or an indirect call can be optimized into a direct
call and error when the inlining fails, but support indirect calls without
errors.

Another case, which is most of the x86/aarch64/arm etc. intrinsics, is
inline always unless there is a target mismatch (roughly what is
actually implemented).

Because from the always_inline attribute it is impossible to determine which
one of those it is (for the indirect calls the rule could be
gnu_inline extern inline means indirect calls are ok, anything else means
indirect calls are bad), we need new syntax to distinguish those cases.

general_regs_only attribute doesn't seem to be it, e.g. for the glibc
fortify wrappers cases I don't see why we should forbid using floating point
in such inlines.

So IMHO we need some new attribute for one of those, or optional parameter
to always_inline.

For the intrinsic case, ideal would be if we could record which ISA flags
(or more generally which options) are required and which are not.  Either
have some syntax where those would be explicitly specified in attribute (but
frankly that would be a maintainance nightmare), or derive those from
surrounding pragmas.  Right now we have those wrapped in
#ifndef __AVX2__
#pragma GCC push_options
#pragma GCC target("avx2")
#define __DISABLE_AVX2__
#endif /* __AVX2__ */

...

#ifdef __DISABLE_AVX2__
#undef __DISABLE_AVX2__
#pragma GCC pop_options
#endif /* __DISABLE_AVX2__ */

The question is if the pragma GCC target right now behaves incrementally
or not, whether
#pragma GCC target("avx2")
adds -mavx2 to options if it was missing before and nothing otherwise, or if
it switches other options off.  If it is incremental, we could e.g. try to
use the second least significant bit of global_options_set.x_* to mean
this option has been set explicitly by some surrounding #pragma GCC target.
The normal tests - global_options_set.x_flag_whatever could still work
fine because they wouldn't care if the option was explicit from anywhere
(command line or GCC target or target attribute) and just & 2 would mean
it was explicit from pragma GCC target; though there is the case of
bitfields... And then the inlining decision could check the & 2 flags to
see what is required and what is just from command line.
Or we can have some other pragma GCC that would be like target but would
have flags that are explicit (and could e.g. be more restricted, to ISA
options only, and let those use in addition to #pragma GCC target.

	Jakub