[PATCH] D107843: [X86] Add parentheses around casts in some of the X86 intrinsic headers.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I found a few more macro hygiene issues in these headers.



================
Comment at: clang/lib/Headers/avx2intrin.h:23
 #define _mm256_mpsadbw_epu8(X, Y, M) \
   (__m256i)__builtin_ia32_mpsadbw256((__v32qi)(__m256i)(X), \
                                      (__v32qi)(__m256i)(Y), (int)(M))
----------------
Parens missing here still


================
Comment at: clang/lib/Headers/avx2intrin.h:1094
 #define _mm256_i64gather_ps(m, i, s) \
   (__m128)__builtin_ia32_gatherq_ps256((__v4sf)_mm_undefined_ps(), \
                                        (float const *)(m), \
----------------
Parens missing here still.


================
Comment at: clang/lib/Headers/smmintrin.h:868-871
 #define _mm_extract_ps(X, N) (__extension__                      \
   ({ union { int __i; float __f; } __t;  \
      __t.__f = __builtin_ia32_vec_ext_v4sf((__v4sf)(__m128)(X), (int)(N)); \
      __t.__i;}))
----------------
This is gross. I wonder if we can use `__builtin_bit_cast` here instead of a cast through a union.



================
Comment at: clang/lib/Headers/smmintrin.h:876
 #define _MM_EXTRACT_FLOAT(D, X, N) \
   { (D) = __builtin_ia32_vec_ext_v4sf((__v4sf)(__m128)(X), (int)(N)); }
 
----------------
The existing code is the wrong way to define a statement-like macro, but I can't find any documentation (anywhere!) for `_MM_EXTRACT_FLOAT` to confirm what the expected valid uses are. The existing formulation would not work in contexts such as:

```
if (1)
  _MM_EXTRACT_FLOAT(d, x, n);
else
  ...
```

... because the semicolon would terminate the `if`, and it would incorrectly work in contexts requiring braces such as:

```
void f(float *pd, __m128 x, int n) _MM_EXTRACT_FLOAT(*pd, x, n)
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107843/new/

https://reviews.llvm.org/D107843