On Thu, Mar 25, 2021 at 6:13 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Thu, Mar 25, 2021 at 02:02:16PM +0100, Uros Bizjak wrote:
> > > Aren't *intrin.h system headers too?
> >
> > I was under impression that they are not, since they live outside of
> > /usr/include.
>
> Yes, they aren't in /usr/include, but they are still system headers.
> If I preprocess something that #include <x86intrin.h> with my system
> compiler, I get:
> # 1 "/usr/lib/gcc/x86_64-redhat-linux/10/include/x86intrin.h" 1 3 4
> where that 3 stands for system header.
>
> My preference would be a new attribute that for always_inline says it is ok
> to inline even when there are target or optimization mismatches (and
> effectively get the target/optimization options from the caller for the
> body) and start using that new attribute in glibc headers (for
> -D_FORTIFY_SOURCE wrappers at least, those really don't have any target
> dependencies nor anything floating point that might e.g. depend on
> -ffast-math etc.) and perhaps the __rdtsc and similar intrinsics in
> *intrin.h.
> Even that can be a can of worms, because some target or optimization options
> are used already in the FE processing or during the GIMPLE passes before
> inlining, and while it might work somehow if e.g. during those passes we
> treat it like -ffast-math and after inlining not like that or vice versa,
> there is a risk that we e.g. fold/lower something with some assumptions and
> later assume that (with different options) such constructs can't appear in
> the IL.
>
> > If the patch does not differentiate between system and user headers,
> > then please revert it.
>
> It does but intrinsic headers are system headers.
>
> Jakub
>
Before my patch:
[hjl@gnu-cfl-2 gcc]$ cat y.c
#include <x86intrin.h>
#ifdef FOO
void
foo (__m512 *p)
{
*p = _mm512_setzero_ps ();
}
#else
__attribute__((target ("avx"))) void
bar (__m512 *p)
{
*p = _mm512_setzero_ps ();
}
#endif
[hjl@gnu-cfl-2 gcc]$ gcc -O2 -S y.c
y.c: In function ‘bar’:
y.c:13:6: warning: AVX512F vector return without AVX512F enabled
changes the ABI [-Wpsabi]
13 | *p = _mm512_setzero_ps ();
| ~~~^~~~~~~~~~~~~~~~~~~~~~
In file included from
/usr/lib/gcc/x86_64-redhat-linux/10/include/immintrin.h:55,
from
/usr/lib/gcc/x86_64-redhat-linux/10/include/x86intrin.h:32,
from y.c:1:
/usr/lib/gcc/x86_64-redhat-linux/10/include/avx512fintrin.h:310:1:
error: inlining failed in call to ‘always_inline’ ‘_mm512_setzero_ps’:
target specific option mismatch
310 | _mm512_setzero_ps (void)
| ^~~~~~~~~~~~~~~~~
y.c:13:8: note: called from here
13 | *p = _mm512_setzero_ps ();
| ^~~~~~~~~~~~~~~~~~~~
[hjl@gnu-cfl-2 gcc]$ gcc -O2 -S y.c -DFOO
y.c: In function ‘foo’:
y.c:7:6: warning: AVX512F vector return without AVX512F enabled
changes the ABI [-Wpsabi]
7 | *p = _mm512_setzero_ps ();
| ~~~^~~~~~~~~~~~~~~~~~~~~~
In file included from
/usr/lib/gcc/x86_64-redhat-linux/10/include/immintrin.h:55,
from
/usr/lib/gcc/x86_64-redhat-linux/10/include/x86intrin.h:32,
from y.c:1:
/usr/lib/gcc/x86_64-redhat-linux/10/include/avx512fintrin.h:310:1:
error: inlining failed in call to ‘always_inline’ ‘_mm512_setzero_ps’:
target specific option mismatch
310 | _mm512_setzero_ps (void)
| ^~~~~~~~~~~~~~~~~
y.c:7:8: note: called from here
7 | *p = _mm512_setzero_ps ();
| ^~~~~~~~~~~~~~~~~~~~
[hjl@gnu-cfl-2 gcc]$
After my patch,
[hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -O2 -S y.c -DFOO
y.c: In function ‘foo’:
y.c:7:6: warning: AVX512F vector return without AVX512F enabled
changes the ABI [-Wpsabi]
7 | *p = _mm512_setzero_ps ();
| ~~~^~~~~~~~~~~~~~~~~~~~~~
In file included from ./include/immintrin.h:49,
from ./include/x86intrin.h:32,
from y.c:1:
./include/avx512fintrin.h:305:1: error: inlining failed in call to
‘always_inline’ ‘_mm512_setzero_ps’: target specific option mismatch
305 | _mm512_setzero_ps (void)
| ^~~~~~~~~~~~~~~~~
y.c:7:8: note: called from here
7 | *p = _mm512_setzero_ps ();
| ^~~~~~~~~~~~~~~~~~~~
[hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -O2 -S y.c
y.c: In function ‘bar’:
y.c:13:6: warning: AVX512F vector return without AVX512F enabled
changes the ABI [-Wpsabi]
13 | *p = _mm512_setzero_ps ();
| ~~~^~~~~~~~~~~~~~~~~~~~~~
[hjl@gnu-cfl-2 gcc]$
If you look at the generated code:
vpxor %xmm0, %xmm0, %xmm0
vmovdqa %xmm0, (%rdi)
vmovdqa %xmm0, 16(%rdi)
vmovdqa %xmm0, 32(%rdi)
vmovdqa %xmm0, 48(%rdi)
ret
The ABI change warning is on _mm512_setzero_ps. Since it is inlined,
there is no wrong code here. I don't believe my patch will cause the wrong
code nor ICE.
--
H.J.
