[PATCH] D21033: Add runtime support for __cpu_model (__builtin_cpu_supports)

[Joerg Sonnenberger via llvm-commits]

joerg added a comment.

I really, really strongly wants to go back to the drawing board with this, including the way the GCC builtin is lowered in first place. There are a number of very big concerns I have and I don't think the current implementation fits:

(1) No constructors in builtins. They introduce fundamental issues for a number of use cases of compiler-rt, like pretty much any self-hosting environment. There is no need either -- for `__builtin_cpu_supports`, only a few global words are needed and the content is idempotent. That means checking if a flag is set and replacing the content is perfectly fine and comes with minimal overhead.

(2) For the purpose of `__builtin_cpu_supports`, only the flags are needed. That means 90% of the code complexity here is unnecessary.

(3) No string interfaces, please. IMO the semantic contract for `__builtin_cpu_supports` should require a string constant as argument, variables should be rejected. The target lowering should then replace the string with the corresponding feature mask. The basic goal here is to avoid having to maintain string->bitmask tables in compiler-rt. I don't want to have to tell people that they need to update central libraries just because someone decided to add a new flag to the list. That approach doesn't scale and the problem of having multiple libgcc_s versions around should be a good indicator of why to avoid this kind of problems.

(4) For the latter work of identifying the CPU, I'm included to make similar requirements. I don't think it belongs into a low-level library like compiler-rt at all, but even then the low-level code should at most expose vendor + cpu family counter. I most explicitly don't want to have an enumeration of all known Intel and AMD CPUs as string table in compiler-rt.


http://reviews.llvm.org/D21033