On Thu, Mar 25, 2021 at 02:21:21PM +0100, Richard Biener wrote:
> Err, but _which_ mismatches do you ignore with such new attribute?
We'd need to define it.
> If I have __rdtsc and compile that into a -mno-rdtsc unit/function would
> that be OK?
There is no -mno-rdtsc or -mrdtsc, rdtsc insn is always available.
So from that POV, having the new attribute on __rdtsc is fine.
The problem with H.J.'s patch is that we'll now accept a lot of code that
should be rejected, we relied for years on the _mm* intrinsics implemented
as always_inline and for it being rejected if users try to use it from
incompatible callers.
Some of them will be still rejected but with different diagnostics (because
users can use the (unsupported) builtins directly, we don't have a guarantee
they aren't used in code with arbitrary ISA flags), some of them will ICE
(if we didn't catch such uses of unsupported builtins, something to be fixed
for sure), and others that are implemented without builtins will be
accepted, which means people will have broken code in the wild that might
then not compile with clang or icc but will compile with gcc.
always_inline simply sometimes means always inline except when it is address
taken and not called directly and at other times ... or when there is a
target or optimization specific option mismatch.
Perhaps a way to make it work most of the time would be to amend
H.J.'s patch to do it only if the callee target options are the default
ones.
That would mean we keep the existing behavior for the
#pragma GCC push_options
#pragma GCC target("avx2")
inline __attribute__((always_inline)) ...
#pragma GCC pop_options
cases and not for the
__fortify_function int
open (const char *__path, int __oflag, ...)
{
...
}
cases. But right now that wouldn't work reliably, because we add those
push_options/target ... only if the currently selected target (e.g. from
command line options) doesn't already include those.
So, we would let the
__attribute__((target ("no-sse3")))
void bar (void) { _mm256_*(); }
cases through.
Another question is what H.J.'s patch will do for LTO with
-fno-early-inlining. If the always_inline functions are only inlined
during LTO inlining, == comparisons with default target option node are just
weird.
Jakub
