[PATCH] D38824: [X86] Synchronize the existing CPU predefined macros with the cases that gcc defines them

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D38824#908461, @chandlerc wrote:

> So, doing research to understand the impact of this has convinced me we *really* need to stop doing this. Multiple libraries are actually trying to enumerate every CPU that has feature X for some feature X. =[[[ This, combined with the fundamental pattern of defining a precise macro for the CPU, leaves a time bomb where anyone that passes a new CPU to `-march` using some older headers will incorrectly believe features aren't available on *newer* CPUs. =[
>
> Specific examples:
>  https://github.com/hwoarang/glibc/blob/master/sysdeps/x86/cpu-features.h#L263
>  https://github.com/boostorg/atomic/blob/boost-1.65.1/include/boost/atomic/detail/caps_gcc_x86.hpp#L30


That's, um, interesting.

OTOH, if there were good feature macros for these things (e.g. cpuid), I imagine that the library authors would have preferred to use them. I doubt the authors of that code really wanted to do it that way either.

> I think my conclusion is that the best way forward is to officially stop defining CPU-specific macros, but to also continue defining __corei7__ macros on all CPUs newer than that for all time so that old headers using these macros for "feature detection" actually work.
> 
> Thoughts?

I think that we do need to at least do that.

One issue is that I've definitely seen the opposite situation as well: Combination of ISA feature macros being used to determine the CPU. That's also awful (and more error prone), and I wouldn't want to push users toward that either. There definitely are situations in which the presence/absence of a feature is not the relevant factor, but the performance of that feature, and so there may be different implementations of an algorithm depending on the identify of the targeted core (not just the ISA features).


https://reviews.llvm.org/D38824