rsmith added inline comments.
================
Comment at: lib/CodeGen/CodeGenModule.cpp:961-970
if (const auto *FD = dyn_cast<FunctionDecl>(ND))
if (FD->isMultiVersion() && !OmitMultiVersionMangling) {
if (FD->isCPUDispatchMultiVersion() || FD->isCPUSpecificMultiVersion())
AppendCPUSpecificCPUDispatchMangling(
CGM, FD->getAttr<CPUSpecificAttr>(), Out);
+ else if (FD->isTargetClonesMultiVersion())
+ AppendTargetClonesMangling(CGM, FD->getAttr<TargetClonesAttr>(), Out);
----------------
erichkeane wrote:
> rsmith wrote:
> > This chain of `if`s and similar things elsewhere make me think we're
> > missing an abstraction. Perhaps `FunctionDecl` should have a
> > `getMultiVersionAttr()` and/or `getMultiVersionKind()` functions?
> I'm not super sure what either buys us... The multiversion attributes are all
> somewhat different unfortunately, so they would need to be dispatched
> separately later. The 'getMultiVersionKind' is perhaps useful, though its
> essentially what isXXXMultiVersion does. I'll think on it, I agree that
> there is likely an abstraction somewhere between that can improve this...
The idea would be that we have an enum that we can perform a covered `switch`
over, so we don't need to remember to update all the relevant places when we
add another kind of multiversioning. You already have the code to work out the
kind of multiversioning; moving it from SemaDecl.cpp to FunctionDecl then using
it where it makes sense in this patch would help, I think.
================
Comment at: lib/Sema/SemaDecl.cpp:9811-9812
// Main isn't allowed to become a multiversion function, however it IS
// permitted to have 'main' be marked with the 'target' optimization hint.
if (NewFD->isMain()) {
----------------
erichkeane wrote:
> rsmith wrote:
> > Why can't `main` be multiversioned? That seems like an arbitrary
> > restriction.
> At the time of implementing 'target', I was unsure of (and still am) how to
> accomplish this. It would seem that I'd need to make the entry point a
> wrapper that calls the ifunc. GCC seems to improperly call this (it doesn't
> emit the 'main' fucntion as far as I can tell).
OK, if there's actually a problem with having the `main` symbol be an ifunc,
that seems like a perfectly legitimate reason for the restriction.
https://reviews.llvm.org/D51650
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
