[PATCH] D67058: [clang][CodeGen] Add alias for cpu_dispatch function with IFunc & Fix resolver linkage type

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3002
         false);
     llvm::Constant *Resolver = GetOrCreateLLVMFunction(
         MangledName + ".resolver", ResolverType, GlobalDecl{},
----------------
zsrkmyn wrote:
> erichkeane wrote:
> > This Resolver should have the same linkage as below.
> Actually, I wanted to set linkage here at the first time, but failed. When compiling code with cpu_specific but no cpu_dispatch, we cannot set it as LinkOnceODR or WeakODR. E.g.:
> 
> ```
> $ cat specific_only.c
> __declspec(cpu_specific(pentium_iii))
> int foo(void) { return 0; }
> int usage() { return foo(); }
> 
> $ clang -fdeclspec specific_only.c                                                 
> Global is external, but doesn't have external or weak linkage!                                                                
> i32 ()* ()* @foo.resolver                                                                                                     
> fatal error: error in backend: Broken module found, compilation aborted!   
> ```
> 
> This is found by lit test test/CodeGen/attr-cpuspecific.c, in which 'SingleVersion()' doesn't have a cpu_dispatch declaration.
The crash message is complaining it isn't external/weak.  However, WeakODR should count, right?  Can you look into it a bit more to see what it thinks is broken?


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3005
         /*ForVTable=*/false);
+    auto Linkage = (FD->isCPUDispatchMultiVersion() || FD->isCPUSpecificMultiVersion())
+        ? llvm::Function::LinkOnceODRLinkage
----------------
zsrkmyn wrote:
> erichkeane wrote:
> > I think this can always just be LinkOnceODR.
> Changing this also changes linkage for attribute(target()), should I also change test cases for them? (including test/CodeGen{,CXX}/attr-*.ll)
Yep!  Also, I think WeakODR is the right one.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67058/new/

https://reviews.llvm.org/D67058