[PATCH] D30738: Don't internalize llvm GV's with InternalizeLinkedSymbols

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D30738#695541, @JDevlieghere wrote:

> In https://reviews.llvm.org/D30738#695504, @tejohnson wrote:
>
> > Can the users of this instead run llvm::internalizeModule on the linked module? Note that this will invoke InternalizePass::internalizeModule, which in turn already has handling for these special values (see the AlwaysPreserved inserts. This routine also adds things in the llvm.used set to the AlwaysPreserved set, which is the correct thing to do, and is missing from the code in the ModuleLinker that is force internalizing everything currently.
>
>
> Won't the pass internalize the whole module rather than only the linked values? Would it make sense to extend the `InternalizePass::internalizeModule` API to take a list of used global variables and call it from the module linker to take care of the internalization?


llvm::internalizeModule takes a callback that can be used to indicate that a GV should not be internalized (MustPreserveGV), so presumably you could use that. I think the right way to do this is from the ModuleLinker clients, not from in the ModuleLinker itself (i.e. remove the InternalizeLinkedSymbols flag and related handling). In fact, you would end up with a circular dependence calling this from the ModuleLinker, since internalizeModule is in Transforms/IPO which has a dependence on Linker.

> I'll add a test if we move forward with the change. My use case is not GPU related, rather something LTO like where we have bitcode modules with clashing function/symbol names but different implementations. The problem is that d we don't know which those will be before linking, so we want to internalize them.

So once they are internalized, the clashing symbols are renamed automatically as they are linked in, I assume? If they are external to start with, how would the linking work in non-LTO mode?


Repository:
  rL LLVM

https://reviews.llvm.org/D30738