lebedev.ri added inline comments.

================
Comment at: lib/Basic/Targets/X86.cpp:169
     setFeatureEnabledImpl(Features, "mpx", true);
     if (Kind != CK_SkylakeServer) // SKX inherits all SKL features, except SGX
       setFeatureEnabledImpl(Features, "sgx", true);
----------------
thiagomacieira wrote:
> craig.topper wrote:
> > craig.topper wrote:
> > > lebedev.ri wrote:
> > > > > high-end desktop and server chips (I can't find any SKX without).
> > > > 
> > > > Are you sure you didn't want to move it into an else of this `if()` ?
> > > > This commit leaves it disabled for all processors, but can be 
> > > > re-enabled for specific builds with -mrtm.
> > > 
> > > So I think this was intended.
> > It's also consistent with gcc based on grepping for _RTM_ in their 
> > predefined macro output. https://godbolt.org/z/dtfrQU They don't infer it 
> > from -mcpu=skylake-avx512, but do when adding -mrtm
> I'm sure I didn't want to :-) Do you want me to do that? And do you want me 
> to do it for IcelakeServer too?
> 
> This matches GCC's behaviour:
> 
> ```
> $ gcc -march=skylake-avx512 -dM -E -xc /dev/null | grep -c RTM
> 0
> ```
> I'm sure I didn't want to :-) Do you want me to do that?

Not really, just looked weird.


https://reviews.llvm.org/D53042



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

Reply via email to