zsrkmyn 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,
----------------
erichkeane wrote:
> erichkeane wrote:
> > 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?
> There must be SOME solution for it, since otherwise inline functions wouldn't
> work. For example:
>
> https://godbolt.org/z/RjB9Xb
>
> Its totally permissible to define and use 'foo::bar' in multiple TUs. Note
> that it is marked linkonce_odr dso_local and comdat. I'm not sure what the
> latter two do, but please experiment and see which will allow the symbol to
> be merged in the linker.
Thanks Erich! Yes, It's the comdat that does the trick. I'll update the patch
later. https://llvm.org/docs/LangRef.html#comdats
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
