[PATCH] D21033: Add runtime support for __cpu_model (__builtin_cpu_supports)
Eric Christopher via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 7 13:53:32 PDT 2016
echristo added a comment.
In http://reviews.llvm.org/D21033#451370, @joerg wrote:
> 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.
Could you explain more? Also talk about how this works with ifunc resolution and uses of __builtin_cpu_is and __builtin_cpu_supports in ifunc resolver functions.
> (2) For the purpose of `__builtin_cpu_supports`, only the flags are needed. That means 90% of the code complexity here is unnecessary.
This isn't just for __builtin_cpu_supports. That's just what Alina needed here.
> (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.
This is talking about a new interface. While I'm somewhat sympathetic to your goals, having a compiler-rt that's a drop in replacement for libgcc unfortunately needs this feature set to be implemented. Also see arguments against implementing ifunc in the first place versus just a (possibly hidden) global that's set at program start time.
More information about the llvm-commits