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,
----------------
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.
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
