erichkeane added a subscriber: rnk.
erichkeane added inline comments.

================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2957
+    if (!AliasFunc) {
+      auto *IFunc = cast<llvm::GlobalIFunc>(GetOrCreateLLVMFunction(
+          AliasName, DeclTy, GD, /*ForVTable=*/false, /*DontDefer=*/true,
----------------
zsrkmyn wrote:
> erichkeane wrote:
> > erichkeane wrote:
> > > I think we want this in GetOrCreateMultiVersionResolver, so that it gets 
> > > created when the ifunc does.  That way you just need a 
> > > "FD->isCPUDispatchMultiVersion() || isCPUSpecificMultiVersion()" check 
> > > inside the supportsIFunc branch.
> > After discussing this offline, I believe this is the right function to 
> > create the alias.  The motivating example is:
> > 
> >     // TU1:
> >     __attribute__((cpu_dispatch(a,b,c))) void foo(void);
> > 
> >     // TU2:
> >     extern void foo(void);
> > 
> > Currently, TU1 doesn't bother to emit the ifunc, because we've attached 
> > emitting this to when this is referenced.
> > 
> > We made that choice because we expected TU2 to mark 'foo' as 
> > cpu_dispatch/cpu_specific in SOME way.  I believe that it is harmless to 
> > emit the ifunc all the time, which this is attempting to do.  However, this 
> > needs to change the ifunc to have LinkOnceODR linkage in 
> > GetOrCreateMultiVersionResolver, otherwise this can cause linker errors.
> > 
> > 
> I've finished most parts. But I think we should also set the resolver to have 
> LinkOnceODR Linkage. Otherwise, we cannot have the cpu_dispatch declaration 
> in multiple TUs.
> 
> However there's no 'weak' symbol on Windows, so even setting the resolver 
> linkage to LinkOnceODR cannot solve the duplicate defined symbol problem on 
> Windows. Do you have any suggestions on it? :-)
Yep, I think the resolver should also be linkonceodr (as well as the ifunc).  
See where they are generated and do it there.

I can't help but think that we've solved the weak symbols issue with windows 
before, but I cannot for the life of me remember.  @rnk  @lebedev.ri , do 
either of you remember?  Does LinkOnceODR not do that trickery?


Repository:
  rC Clang

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