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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to