Skip to content

Commit

Permalink
[ThinLTO][LowerTypeTests] Don't compute address taken set unless CFI …
Browse files Browse the repository at this point in the history
…(NFC) (llvm#118508)

The AddressTaken set used for CFI with regular LTO was being computed on
the ExportSummary regardless of whether any CFI metadata existed. In the
case of ThinLTO, the ExportSummary is the global summary index for the
target, and the lack of guard in this code meant this was being computed
on the ThinLTO index even when there was an empty regular LTO module,
since the backend is called on the combined module to generate the
expected output file (normally this is trivial as there is no IR).

Move the computation of the AddressTaken set into the condition checking
for CFI to avoid this overhead. This change resulted in a 20% speedup in
the thin link of a large target. It looks like the outer loop has
existed here for several years, but likely became a larger overhead
after the inner loop was added very recently in PR113987.

I will send a separate patch to refactor the ThinLTO backend handling to
avoid invoking the opt pipeline if the module is empty, in case there
are other summary-based analyses in some of the passes now or in the
future. This change is still desireable as by default regular LTO
modules contain summaries, or we can have split thin and regular LTO
modules, and if they don't involve CFI these would still unnecessarily
compute the AddressTaken set.
  • Loading branch information
teresajohnson authored Dec 3, 2024
1 parent 2d57333 commit d6cd214
Showing 1 changed file with 11 additions and 12 deletions.
23 changes: 11 additions & 12 deletions llvm/lib/Transforms/IPO/LowerTypeTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2090,20 +2090,19 @@ bool LowerTypeTestsModule::lower() {
};
MapVector<StringRef, ExportedFunctionInfo> ExportedFunctions;
if (ExportSummary) {
// A set of all functions that are address taken by a live global object.
DenseSet<GlobalValue::GUID> AddressTaken;
for (auto &I : *ExportSummary)
for (auto &GVS : I.second.SummaryList)
if (GVS->isLive())
for (const auto &Ref : GVS->refs()) {
AddressTaken.insert(Ref.getGUID());
for (auto &RefGVS : Ref.getSummaryList())
if (auto Alias = dyn_cast<AliasSummary>(RefGVS.get()))
AddressTaken.insert(Alias->getAliaseeGUID());
}

NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions");
if (CfiFunctionsMD) {
// A set of all functions that are address taken by a live global object.
DenseSet<GlobalValue::GUID> AddressTaken;
for (auto &I : *ExportSummary)
for (auto &GVS : I.second.SummaryList)
if (GVS->isLive())
for (const auto &Ref : GVS->refs()) {
AddressTaken.insert(Ref.getGUID());
for (auto &RefGVS : Ref.getSummaryList())
if (auto Alias = dyn_cast<AliasSummary>(RefGVS.get()))
AddressTaken.insert(Alias->getAliaseeGUID());
}
for (auto *FuncMD : CfiFunctionsMD->operands()) {
assert(FuncMD->getNumOperands() >= 2);
StringRef FunctionName =
Expand Down

0 comments on commit d6cd214

Please sign in to comment.