[X86][bugfix] redefine __SSC_MARK to escape cpp string literal concatenation problem

[xiangzhangllvm]

Summary

Redefine __SSC_MARK to escape cpp string literal concatenation problem.

This patch is to refine D129826
The old definition #define __MOVEGPR(__r1, __r2) "mov {%%"__r1 ", %%"__r2 "|"__r2 ", "__r1"};" is not support in c++11.

So this patch conservatively re-implement it.

For example:
The old way will cause build c++ file test.cpp fail:

#include <immintrin.h>
int main()
{
    __SSC_MARK(222);
    return 0;
}

clang test.cpp -std=c++11 -S

test.cpp:7:5: error: differing user-defined suffixes ('__r1' and '__r2') in string literal concatenation
    __SSC_MARK(222);

[RKSimon]

clang/lib/Headers/x86gprintrin.h

39

I assume all these defines are for internal use and only __SSC_MARK should be public?

In which case should the bottom of the header have something like this:

#undef __TMPGPR
#undef __SAVE_GPRBX
#undef __RESTORE_GPRBX

[xiangzhangllvm]

clang/lib/Headers/x86gprintrin.h

39

This will meet problem, Because these definitions in *.h file, the source file include it before.

so if we undef them like:

#define AA
#define BB AA
...
#undef AA
...
use BB  // The AA has been undef

Thank you!

[RKSimon]

clang/lib/Headers/x86gprintrin.h

39

Sorry - wasn't thinking that one through properly!

What about repeating the __SSC_MARK define - for 32/64 bit cases and without the other defines?

[xiangzhangllvm]

clang/lib/Headers/x86gprintrin.h

39

The user should make sure not repeating the __SSC_MARK when they include the x86gprintrin.h.
We have a lot of similar macro definition in clang/lib/Headers/*.h (e.g. xmmintrin.h::#define _MM_SHUFFLE)
Thanks!

[xiangzhangllvm]

ping, The patch is small : )

[pengfei]

LGTM.

[pengfei]

Given it is only a c++ problem, it is also possible to solve it by using extern "C" { ... }? We use this method in some headers.

[xiangzhangllvm]

In D132636#3760256, @pengfei wrote:

Given it is only a c++ problem, it is also possible to solve it by using extern "C" { ... }? We use this method in some headers.

I tried it before, it doesn't works for macro literal concatenation, I think the extern "C" { ... } mainly work for name decoration （e.g. Function signature）
Thanks for your reviewing!

clang/lib/Headers/x86gprintrin.h

 #endif
 
 #if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||      \
     defined(__CRC32__)
 #include <crc32intrin.h>
 #endif
 
 #if defined(__i386__)
-#define __FULLBX "ebx"
+#define __SAVE_GPRBX "mov {%%ebx, %%eax |eax, ebx};"
+#define __RESTORE_GPRBX "mov {%%eax, %%ebx |ebx, eax};"
 #define __TMPGPR "eax"
 #else
 // When in 64-bit target, the 32-bit operands generate a 32-bit result,
 // zero-extended to a 64-bit result in the destination general-purpose,
 // It means "mov x %ebx" will clobber the higher 32 bits of rbx, so we
 // should preserve the 64-bit register rbx.
-#define __FULLBX "rbx"
+#define __SAVE_GPRBX "mov {%%rbx, %%rax |rax, rbx};"
+#define __RESTORE_GPRBX "mov {%%rax, %%rbx |rbx, rax};"
 #define __TMPGPR "rax"
 #endif
 
-#define __MOVEGPR(__r1, __r2) "mov {%%"__r1 ", %%"__r2 "|"__r2 ", "__r1"};"
-
-#define __SAVE_GPRBX __MOVEGPR(__FULLBX, __TMPGPR)
-#define __RESTORE_GPRBX __MOVEGPR(__TMPGPR, __FULLBX)
-
 #define __SSC_MARK(__Tag)                                                      \
   __asm__ __volatile__( __SAVE_GPRBX                                           \
                        "mov {%0, %%ebx|ebx, %0}; "                             \
                        ".byte 0x64, 0x67, 0x90; "                              \
                         __RESTORE_GPRBX                                        \
                        ::"i"(__Tag)                                            \
                        :  __TMPGPR );
 
 #endif /* __X86GPRINTRIN_H */