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(); ``` https://reviews.llvm.org/D47474 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits