[PATCH] D17090: Fix LLVM's handling and detection of skylake CPUs

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 13 08:52:40 PST 2016


spatel added subscribers: RKSimon, qcolombet, echristo, zansari.
spatel added a comment.

Craig proposed a change in the post-commit thread for r258659 which may affect this patch. I agree with this:
"I don't like the idea of these wrapper processor feature sets. I don't like the SLM and Atom ones we already had. It always felt like the checks that were based on Atom/SLM should be based on -mcpu or -march and not a "feature". I don't think its a good idea to encourage more of this."

I would go even further: basing any codegen transform on a CPU model rather than some specific feature of that microarch is bad. Basing a code transform on a CPU's marketing name is double-bad. :)

Sooner or later, that kind of predicate will need fixing.

I think it's fine to have micro-arch and marketing name bundles up in clang, but I'd like to see X86ProcFamilyEnum disappear down here.

Related: we have a bunch of "FeatureSlow*" bits which aren't really features either. And although I've added to that collection myself, I think it would be better to sink some of those transforms to the machine instruction level. There was some discussion about that here:
https://llvm.org/bugs/show_bug.cgi?id=26183
Alternatively, we could do more DAG-level predication like this:
http://reviews.llvm.org/rL244601
or hoist things up to CodeGenPrepare and use TTI/TLI.


http://reviews.llvm.org/D17090





More information about the llvm-commits mailing list