zsrkmyn added inline comments.

================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3002
         false);
     llvm::Constant *Resolver = GetOrCreateLLVMFunction(
         MangledName + ".resolver", ResolverType, GlobalDecl{},
----------------
erichkeane wrote:
> zsrkmyn wrote:
> > zsrkmyn wrote:
> > > erichkeane wrote:
> > > > 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?
> > > No, actually I've tried it earlier with the example I mentioned in my 
> > > last comment, but WeakODR still makes compiler complaining. I think it's 
> > > `foo.resolver` that cannot be declared with as WeakODR/LinkOnceODR 
> > > without definition. But I'm really not familiar with these rules.
> > According to the `Verifier::visitGlobalValue()` in Verify.cpp, an 
> > declaration can only be `ExternalLinkage` or `ExternalWeakLinkage`. So I 
> > still believe we cannot set the resolver to 
> > `LinkOnceODRLinkage/WeakODRLinkage` here, as they are declared but not 
> > defined when we only have `cpu_specified` but no `cpu_dispatch` in a TU as 
> > the example above.
> That doesn't seem right then.  IF it allows ExternalWeakLinkage I'd expect 
> WeakODR to work as well, since it is essentially the same thing.
I think we should have a double check. It is said "It is illegal for a function 
declaration to have any linkage type other than `external` or `extern_weak`" at 
the last line of section Linkage Type in the reference manual [1]. I guess 
`weak_odr` is not designed for declaration purpose and should be only used by 
definition.

[1] https://llvm.org/docs/LangRef.html#linkage-types


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