clang, C++ and compound literals in SSE2 intrinsics

Hello,

compiling some SSE2 code with clang in C++0x mode and "-pedantic", I got the following warning:
"Compound literals are a C99-specific feature". I looked into the file "emmintrin.h" coming with
clang, where the warning originated, and _mm_shuffle_epi32 (as well as some other intrinsics)
indeed uses compound literals (emphasis mine):

#define _mm_shuffle_epi32(a, imm) \
  ((__m128i)__builtin_shufflevector((__v4si)(a), (__v4si) {0}, \
                                                 ^^^^^^^^^^^^
                                    (imm) & 0x3, ((imm) & 0xc) >> 2, \
                                    ((imm) & 0x30) >> 4, ((imm) & 0xc0) >> 6))

The code works correctly, so clang is able to compile it. Still, I'd like to get rid of the false
warning. IMO there are two possibilities:

1. Change emmintrin.h so that the intrinsics are implemented without compound literals (if
   possible)

2. Change emmintrin.h so that the compound literal warning is suppressed just for that file.

Unfortunately, I don't know to perform either change. I'm not even sure if (2) is currently
possible with clang at all.

What do you think?

Jonathan

Hi Jonathan,

Warnings are disabled for system headers in general. The issue here is that he warning is coming from a macro that is instantiated from a system header. There are two more solutions:

1a: Fix #defines to avoid compound literals.

3. Change the compound literal warning to not fire if the expansion is from a system header.

I'd prefer 1a.

-Chris

Hello,

Warnings are disabled for system headers in general. The issue here is that he warning is coming from a macro that is instantiated from a system header. There are two more solutions:

1a: Fix #defines to avoid compound literals.

3. Change the compound literal warning to not fire if the expansion is from a system header.

I'd prefer 1a.

Well, I looked a bit further into the header and attempted to implement 1a. I tested _mm_shuffle_epi32 in
a simple test program, and the code generated by clang was the same before and after[*]. I have attached a
patch (diff in directory tools/clang).

As seen in the diff, I simply changed the compound literal to the appropriate _mm_set_* function. Another
possibility would be to change _mm_shuffle_* from a macro to a function, but I assume there is a reason it
is currently implemented as a macro and not, as most of the other intrinsics, a function. So I'm a bit
reluctant here.

Perhaps someone with more SSE experience than I can look into it.

Jonathan

[*] Unfortunately, I cannot test the change with the code that produced the warning in the first place, as I
replaced it just today with OpenGL-based instancing ...

emmintrin.diff (1.32 KB)

Patch looks great, applied in r130149, thanks! These must be macros because __builtin_shufflevector requires that the indices be constants.

-Chris

I wrote:

As seen in the diff, I simply changed the compound literal to the appropriate _mm_set_* function. Another
possibility would be to change _mm_shuffle_* from a macro to a function, but I assume there is a reason it
is currently implemented as a macro and not, as most of the other intrinsics, a function. So I'm a bit
reluctant here.

The reason for the macro is that the indices passed to __builtin_shufflevector must be integer constants,
and function arguments aren't.

(I looked into SVN's history for emmintrin.h, and the three macros in question have been implemented as
they are now in revision 72979, apparently when __builtin_shufflevector was introduced to replace the
builtins __builtin_ia32_pshufd, __builtin_ia32_pshufhw and __builtin_ia32_pshuflw.)

So it seems that using _mm_set_* is the way to go.

Jonathan