[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
Erich Keane via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 9 08:03:02 PDT 2018
erichkeane marked 4 inline comments as done.
erichkeane added a comment.
In https://reviews.llvm.org/D47474#1154858, @lebedev.ri wrote:
> Some drive-by nits.
> General observation: this is kinda large.
> I would //personally// split split off the codegen part into a second patch.
Thanks for the review! I definitely agree this is a sizable patch, I'm not sure the non-codegen part has any value without the codegen so I'm not sure that splitting it would provide value. I can definitely do so if reviewers in general believe this is the right tack.
Comment at: include/clang/Basic/X86Target.def:288
+// FIXME: When commented out features are supported in LLVM, enable them here.
+CPU_SPECIFIC("generic", 'A', "")
> This is going to go stale.
> It already does not have znver1, btver2.
> Surely this info already exists elsewhere in llvm?
> Can it be deduplicated somehow?
You'll note that this doesn't have any non-intel names :) This is non-Intel processor friendly version of the ICC implementation of the same feature. Because of this, I don't have a good feature list for those.
I'd searched in a few places for an alternate source for these names/feature lists. I was unable to find anywhere else that needs/has similar information, though any alternate sources are greatly appreciated.
I'll note that the name and mangling-name aren't going to be able to be removed, though perhaps there is a source of these features somewhere?
Comment at: lib/CodeGen/CodeGenModule.cpp:868
+ const auto &Target = CGM.getTarget();
+ return std::string(".") + Target.CPUSpecificManglingCharacter(Name);
> I think
> return Twine(".", Target.CPUSpecificManglingCharacter(Name)).str();
A twine of char + a char is actually pretty challenging... I rewrote it without the intermediate string though. I am generally unconcerned, since 2 chars is very much in the 'short string optimization' range of all implementations.
More information about the cfe-commits