Patch RFA: Move x86 _mm_pause out of pragma target("sse") scope

[Ian Lance Taylor]

The _mm_pause intrinsic is defined in xmmintrin.h. Right now using it
with -m32 with the default -march option gives an error:

/home/iant/foo.c: In function ‘f’:
/home/iant/gcc/go-install/lib/gcc/x86_64-unknown-linux-gnu/4.10.0/include/xmmintrin.h:1238:1: error: inlining failed in call to always_inline ‘_mm_pause’: target specific option mismatch
_mm_pause (void)
^
/home/iant/foo5.c:3:13: error: called from here
void f () { _mm_pause (); }
^

This error is because _mm_pause is defined in the scope of #pragma GCC
target("sse"). But _mm_pause, which simply generates the pause
instruction, does not require SSE support. The pause instruction has
nothing really to do with SSE, and it works on all x86 processors (on
processors that do not explicitly recognize it, it is a nop).

I propose the following patch, which moves _mm_pause out of the pragma
target scope.

I know that x86intrin.h provides a similar intrinsic, __pause, but I
think it's worth making _mm_pause work reasonably as well.

I'm running a full testsuite run. OK for mainline if it passes?

Ian


gcc/ChangeLog:

2014-05-29 Ian Lance Taylor <***@google.com>

* config/i386/xmmintrin.h (_mm_pause): Move out of scope of pragma
target("sse").

gcc/testsuite/ChangeLog:

2014-05-29 Ian Lance Taylor <***@google.com>

* gcc.target/i386/pause-2.c: New test.

[Uros Bizjak]

Hello!

Post by Ian Lance Taylor
This error is because _mm_pause is defined in the scope of #pragma GCC
target("sse"). But _mm_pause, which simply generates the pause
instruction, does not require SSE support. The pause instruction has
nothing really to do with SSE, and it works on all x86 processors (on
processors that do not explicitly recognize it, it is a nop).
I propose the following patch, which moves _mm_pause out of the pragma
target scope.
I know that x86intrin.h provides a similar intrinsic, __pause, but I
think it's worth making _mm_pause work reasonably as well.
I'm running a full testsuite run. OK for mainline if it passes?
* config/i386/xmmintrin.h (_mm_pause): Move out of scope of pragma
target("sse").
* gcc.target/i386/pause-2.c: New test.

The patch looks OK to me, but please wait a day or two for possible
comments (compatibility, etc) from Kirill.

Thanks,
Uros.

[Kirill Yukhin]

Hello Ian, Uroš,

Post by Uros Bizjak
Hello!

Post by Ian Lance Taylor
This error is because _mm_pause is defined in the scope of #pragma GCC
target("sse"). But _mm_pause, which simply generates the pause
instruction, does not require SSE support. The pause instruction has
nothing really to do with SSE, and it works on all x86 processors (on
processors that do not explicitly recognize it, it is a nop).
I propose the following patch, which moves _mm_pause out of the pragma
target scope.
I know that x86intrin.h provides a similar intrinsic, __pause, but I
think it's worth making _mm_pause work reasonably as well.
I'm running a full testsuite run. OK for mainline if it passes?
* config/i386/xmmintrin.h (_mm_pause): Move out of scope of pragma
target("sse").
* gcc.target/i386/pause-2.c: New test.

The patch looks OK to me, but please wait a day or two for possible
comments (compatibility, etc) from Kirill.

That is definetely a bug and I see no compatibility issues in the fix.

The only nit I see: maybe it'd be better to put this cpuid-less intrinsic
into immintin.h? xmmintrin.h serves for SSE.

--
Thanks, K

[Jakub Jelinek]

Post by Kirill Yukhin
That is definetely a bug and I see no compatibility issues in the fix.
The only nit I see: maybe it'd be better to put this cpuid-less intrinsic
into immintin.h? xmmintrin.h serves for SSE.

Wouldn't that be an API compatibility problem?
I mean, xmmintrin.h header isn't a new header that can be only included
as part of immintrin.h or x86intrin.h, and people who include xmmintrin.h
and expect to find _mm_pause would suddenly get compiler errors.

Jakub

[Kirill Yukhin]

Post by Jakub Jelinek

Post by Kirill Yukhin
That is definetely a bug and I see no compatibility issues in the fix.
The only nit I see: maybe it'd be better to put this cpuid-less intrinsic
into immintin.h? xmmintrin.h serves for SSE.

Wouldn't that be an API compatibility problem?
I mean, xmmintrin.h header isn't a new header that can be only included
as part of immintrin.h or x86intrin.h, and people who include xmmintrin.h
and expect to find _mm_pause would suddenly get compiler errors.

Makes sense. Moreover we already have cpuid-less _mm_prefetch () in that
header. So, I take it back.

--
Thanks, K