[PATCH] D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode.

JF Bastien via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 13 13:36:56 PDT 2019


jfb added a comment.

In D59287#1427945 <https://reviews.llvm.org/D59287#1427945>, @craig.topper wrote:

> Is this ok with the backend fixed?


This is definitely better.

> Or do you want me factor this into HasCX16 which I think is only used by the defineMacro and the return for hasFeature("cx16")? And I think hasFeature("cx16") is only used by that getMaxAtomicWidth() code which is only called on 64 bit.
> 
> Or we could maybe ignore "cx16" in setFeatureEnabled on 32 bit targets? But I think that would break always_inline on a target attribute with cx16 in 32 bit mode which gcc does allow. https://godbolt.org/z/TW985s

I'm not sure. Does clang ever error out when you have inconsistent platform features and arch on the command line? That seems like what we should be doing here, no?
Because your change just hides a mistake, and clang is usually the only place where we catch mistakes (the rest of LLVM can't diagnose).


Repository:
  rC Clang

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

https://reviews.llvm.org/D59287





More information about the llvm-commits mailing list