On Thu, Mar 25, 2021 at 1:54 PM Jakub Jelinek <[email protected]> wrote:
>
> On Wed, Mar 24, 2021 at 10:23:44AM -0700, H.J. Lu via Gcc-patches wrote:
> > For always_inline in system headers, we don't know if caller's ISAs are
> > compatible with callee's ISAs until much later. Skip ISA check for
> > always_inline in system headers if caller has target attribute.
> >
> > gcc/
> >
> > PR target/98209
> > PR target/99744
> > * config/i386/i386.c (ix86_can_inline_p): Don't check ISA for
> > always_inline in system headers.
>
> Aren't *intrin.h system headers too?
I was under impression that they are not, since they live outside of
/usr/include.
> Doesn't this mean we can now inline all the intrinsics if the caller doesn't
> have the default target options and doesn't have the needed ISA?
No, we should not.
> Consider e.g.
> #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
>
> #ifdef FOO
> void
> baz (__m512d *p, __m512d *q, int mask)
> {
> *p = _mm512_mask_mov_pd (*p, mask, *q);
> }
> #else
> __attribute__((target ("avx"))) void
> qux (__m512d *p, __m512d *q, int mask)
> {
> *p = _mm512_mask_mov_pd (*p, mask, *q);
> }
> #endif
>
> If you compile this without your patch, you'll get
> inlining failed in call to ‘always_inline’ ‘_mm512_setzero_ps’: target
> specific option mismatch
> errors in all cases (always the first one), but with your patch
> the _mm512_setzero_ps (); gets through completely and on the mask move
> one gets instead
> ‘__builtin_ia32_movapd512_mask’ needs isa option -mavx512f
> error and
> the ABI for passing parameters with 64-byte alignment has changed in GCC 4.6
> note. IMNSHO this change needs to be reverted and we need to come up with a
> way (some attribute) to say explicitly whether we can or can't inline that
> always_inline function despite target specific option mismatches.
If the patch does not differentiate between system and user headers,
then please revert it.
Uros.
