[PATCH] D23488: ThinLTO: add early "dead-stripping" on the Index

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In https://reviews.llvm.org/D23488#627314, @tejohnson wrote:

> In https://reviews.llvm.org/D23488#627265, @pcc wrote:
>
> > In https://reviews.llvm.org/D23488#627223, @tejohnson wrote:
> >
> > > In https://reviews.llvm.org/D23488#627220, @tejohnson wrote:
> > >
> > > > We don't want to parse the module and invoke collectUsedGlobalVariables when we do the thin link, which is where we need this info. Oh - I see that we are currently doing this. ISTR that this is only temporary (I hope - we don't want to have to parse the IR in the ThinLink!).
> > >
> > >
> > > Actually, looking at how this info is used right now, it seems like what I am talking about doing here will allow us to remove the build of the module and invocation of collectUsedGlobalVariables from addThinLTO. It is currently being used to help identify exported GUIDs to prevent internalization, but by marking these as live roots in the summary we can do the same thing: treating the flagged live roots as potentially exported and not dead.
> >
> >
> > I don't think that just removing the call to collectUsedGlobalVariables would be sufficient to avoid creating the module. We currently still need the module to create symbol names and compute module flags for the LTO client.
>
>
> The symbol name creation is down in the InputFile and will go away with the new bitcode symbol table, right?


Right.

> Where are the module flags needed?

Sorry, I meant the symbol flags (i.e. `ModuleSymbolTable::getSymbolFlags`).

> 
> 
>> 
>> 
>>>> So it would have to be done during the compile step (where we could do what you suggest above). And then we put it in the summary. Although like I mentioned above, we can just flag the llvm.* variables as the live roots, that's simpler than walking all of their references and flagging them.
>>> 
>>> To do what I describe above (use the summary flagging to obviate the collectUsedGlobalVariables invocation from the thin link), we'll need to flag the references of the llvm.* variables.
>> 
>> Aren't the llvm.* variables and their references available in the summary?
> 
> Yes
> 
>> Presumably even once we move to the bitcode symbol table we could treat llvm.* as GC roots by following their references, no?
> 
> For the computeDeadSymbols this works. But what that doesn't get us is treating those as external (subsuming the handling of the Used variables in addSymbolToGlobalRes), so that they aren't internalized.

Sure, that's what I was thinking about here:

> I was thinking about whether we'd also need a separate "used" bit in the bitcode symbol table, so that we can apply the right semantics to those symbols in both regular and thin LTO. In that case we'd actually want to mark the globals referenced by llvm.used rather than just marking llvm.used itself (because the refs wouldn't be available in the BC symtab). But I think that's probably best dealt with orthogonally.


https://reviews.llvm.org/D23488