[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 6 15:17:24 PDT 2018

lebedev.ri added a comment.

Some drive-by nits.
General observation: this is kinda large.
I would //personally// split split off the codegen part into a second patch.

Comment at: include/clang/AST/Decl.h:2212-2213
+  bool isCPUDispatchMultiVersion() const;
+  bool isCPUSpecificMultiVersion() const;
Everything else has comments, and i'm not sure it is obvious what those mean without knowing the context.

Comment at: include/clang/Basic/AttrDocs.td:223
+A dispatching (or resolving) function can be declared anywhere in your
+compilation with ``cpu_dispatch``. This attribute takes one or more CPU names
+as a parameter (like ``cpu_specific``). Functions marked with ``cpu_dispatch``
s/compilation/source code/?

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?

Comment at: lib/CodeGen/CodeGenFunction.cpp:2421
+  // Main function's basic block.
+  llvm::BasicBlock *CurBlock = createBasicBlock("entry", Resolver);
+  Builder.SetInsertPoint(CurBlock);
Those are arbitrary names, right?
Maybe somehow convey that this is CPUDispatchMultiVersionResolver logic in the names?

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();


More information about the cfe-commits mailing list