Refining which symbols are preemptable with lto

As a follow up to https://reviews.llvm.org/D20217 I would like to use
lto/thinlto to refine when a symbol is marked as local/preemptable.
I'm not very familiar with lto though so would appreciate some
guidance about how best to go about this.

Regards
Sean Fertile

As a follow up to https://reviews.llvm.org/D20217 I would like to use
lto/thinlto to refine when a symbol is marked as local/preemptable.
I'm not very familiar with lto though so would appreciate some
guidance about how best to go about this.

Hi Sean,

The first thing that I would do is to try to get this working for regular
LTO. There is a flag in the lto::SymbolResolution class named
FinalDefinitionInLinkageUnit, which was intended to control whether the
backend must make symbol references preemptible. Right now we copy a number
of other fields from SymbolResolution onto the symbol definition, which is
done by LTO::addRegularLTO in this loop:
https://github.com/llvm-project/llvm-project-20170507/blob/4240587b0d490325dae8aa64d620ce599fa20840/llvm/lib/LTO/LTO.cpp#L555
I imagine that this is where you would add code that translates
FinalDefinitionInLinkageUnit into dso_local/dso_preemptible.

The next step would be to get it working for ThinLTO. This would involve
the same basic mechanism: the FinalDefinitionInLinkageUnit flag would
control whether to set dso_local/dso_preemptible on global values. The
difference would be that the flag would need to be passed via the module
summary index, as this is how information needs to be passed to the ThinLTO
backends. The way that I imagine that this would work is that we would
introduce a new flag in GlobalValueSummary::GVFlags into which you would
copy the value of FinalDefinitionInLinkageUnit during LTO::addThinLTO. We
already copy the Prevailing flag elsewhere using the GUID:
https://github.com/llvm-project/llvm-project-20170507/blob/4240587b0d490325dae8aa64d620ce599fa20840/llvm/lib/LTO/LTO.cpp#L578
but for this flag I imagine that you would want to copy it directly into
the global's summary (see ModuleSummaryIndex::getValueInfo for how to look
up a global value summary using a GUID).

The default value of this flag should mean "preemptible" -- Apple's linker
uses a legacy API which will not set this flag, so that avoids breaking
their linker.

Then in the thinBackend function you would translate the flag into
dso_local/dso_preemptible. Probably the best place to do that is
in FunctionImportGlobalProcessing::processGlobalForThinLTO here:
https://github.com/llvm-project/llvm-project-20170507/blob/4240587b0d490325dae8aa64d620ce599fa20840/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp#L215
See shouldPromoteLocalToGlobal for how to find the summary for the
GlobalVariable at that point -- some of that code probably needs to be
factored out.

There would also need to be bitcode reader/writer support for the new flag:
https://github.com/llvm-project/llvm-project-20170507/blob/4240587b0d490325dae8aa64d620ce599fa20840/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L865
https://github.com/llvm-project/llvm-project-20170507/blob/4240587b0d490325dae8aa64d620ce599fa20840/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp#L879
There is one subtlety however, which is that the summary index might not
contain entries for functions without definitions. For now I would punt on
this and mark declarations that are not present in the summary index as
preemptible.

Peter

Thank you for such detailed instructions Peter!