[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:
> 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.


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

https://reviews.llvm.org/D67058