[PATCH] D20988: [cpu-detection] Substantial refactor of Host CPU detection code (x86)

[Joerg Sonnenberger via llvm-commits]

joerg added a subscriber: joerg.
joerg added a comment.

Do we really want to have this in compiler-rt? It seems to me like the kind of code that is going to be a huge maintenance hassle, especially for Operating Systems that want to support code for longer than a couple of month. Just like the corresponding feature set builtins with string arguments, this seems to me like a horrible API design for a library that is potentially embedded in every binary. The suggested use cases in the GCC documentation don't feel very promising to me either.

As I mentioned on IRC, I can understand wanting to easily test for feature bits, but that's much better provided by having a `__builtin_cpuid_feature(n)` helper and just using the stable and documented bitmasks directly. It would be useful to know if people are just using this kind of checks in initialization code to select dispatcher functions (or the equivalent ifunc resolvers ) or whether they are used in actual hot code. Depending on that, caching the cpuid result is import or not. Without a need to cache cpuid results, `__builtin_cpu_supports` can be expanded completely inline without any library support. If there is a certain desire for efficiency here, being able to merge multiple feature tests could be helpful as well. In any case, I don't believe that forcing string matching into an API for something that is using bitmasks or enumerations on the hardware side anyway is just silly. Doing the string translations completely on the LLVM/Clang side would at least stop that maintenance hassle.


http://reviews.llvm.org/D20988