[cfe-dev] [PROPOSAL] Reintroduce guards for Intel intrinsic headers

[Reid Kleckner]

I'm opposed to this. Going forward, I would really like target intrinsics
to be available regardless of the current feature set, so users don't need
hacks like these.

I see two ways to do this with different tradeoffs:
1. Diagnose missing target attributes when calling the intel intrinsics. I
was surprised to find that we don't already do this.
2. We could support some automatic transfer of the target attribute to the
caller when calling these intrinsics, but I worry that this is too
confusing.

Implicitly setting a target attribute may block inlining that the user
expected to happen, for example. Alternatively, there may be a dynamic
cpuid check in the same function between SSE2 and AVX variants of the same
algorithm, and now the SSE2 loop will unexpectedly use AVX instructions.

So we should probably settle with telling the user to add -msseNN or
__atribute__((target(("sseNN")))).

On Thu, Jul 30, 2015 at 8:53 AM, Vedant Kumar <vsk at apple.com> wrote:

> I've run into some code which no longer compiles because of two recent
> changes:
>
>   41885d3 Update the intel intrinsic headers to use the target attribute
> support.
>   695aff1 Use a define for per-file function attributes for the Intel
> intrinsic headers.
>
> Specifically, one project defines its own SSE4.1 emulation routines when
> the real intrinsics aren't available. This is a problem because they've
> reused the names of the intrinsics. E.g;
>
> > #ifndef __SSE4_1__
> > #define _mm_extract_epi8(a_, ndx) ({ ... })
> > static inline __m128i _mm_blendv_epi8(__m128i a, __m128i b, __m128i
> mask) { ... }
> > ...
> > #endif
>
> SSE4.1 intrinsics now leak into the project when it's being compiled for
> targets without SSE4.1 support. Compilation fails with "error: redefinition
> ...".
>
> When these changes were initially being discussed, I think our stance was
> that we shouldn't support code like this [1]. However, we should reconsider
> for the sake of avoiding breakage. AFAICT, we would need to revert just two
> types of changes:
>
> In lib/Headers/__wmmintrin_aes.h:
>
> > -#if defined (__SSE4_2__) || defined (__SSE4_1__)
> >  #include <smmintrin.h>
> > -#endif
>
>
> In lib/Headers/smmintrin.h:
>
> > -#ifndef __SSE4_1__
> > -#error "SSE4.1 instruction set not enabled"
> > -#else
>
> I don't see any downsides to reintroducing these guards. If everyone's OK
> with this, I can mail a patch in. The alternative is to have clients
> rewrite their emulation layers like this:
>
> > #ifdef __SSE4_1__
> > #define compat_mm_extract_epi8 _mm_extract_epi8
> > static inline __m128i combat_mm_blendv_epi8(__m128i a, __m128i b,
> __m128i mask) __attribute__((__target__(("sse4.1")))) {
> >   return _mm_blendv_epi8(a, b, mask);
> > }
> > ...
> > #else /* OK, no native SSE 4.1. Define our own. */
> > #define compat_mm_extract_epi8(a_, ndx) ({ ... })
> > static inline __m128i compat_mm_blendv_epi8(__m128i a, __m128i b,
> __m128i mask) { ... }
> > ...
> > #endif
>
> ... and then replace all calls to intrinsics with calls to the new
> compatibility routines. This seems like a lot of tedious work, and I'd love
> to help people avoid it :).
>
> Let me know what you think!
>
> vedant
>
> [1]
> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150615/131192.html
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150730/5bebb9a0/attachment.html>