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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to