This is an archive of the discontinued LLVM Phabricator instance.

[ThinLTO] Thin link efficiency: More efficient export list computation
ClosedPublic

Authored by tejohnson on Dec 14 2016, 7:50 AM.

Details

Summary

Instead of checking whether a global referenced by a function being
imported is defined in the same module, speculatively always add the
referenced globals to the module's export list. After all imports are
computed, for each module prune any not in its defined set from its
export list.

For a huge C++ app with aggressive importing thresholds, even with
D27687 we spent a lot of time invoking modulePath() from
exportGlobalInModule (modulePath() was still the 2nd hottest routine in
profile). The reason is that with comdat/linkonce the summary lists for
each GUID can be long. For the app in question, for example, we were
invoking exportGlobalInModule almost 2 million times, and we traversed
an average of 63 entries in the summary list each time.

This patch reduced the thin link time for the app by about 10% (on top
of D27687) when using aggressive importing thresholds, and about 3.5% on
average with default importing thresholds.

Diff Detail

Repository
rL LLVM

Event Timeline

tejohnson updated this revision to Diff 81380.Dec 14 2016, 7:50 AM
tejohnson retitled this revision from to [ThinLTO] Thin link efficiency: More efficient export list computation.
tejohnson updated this object.
tejohnson added a reviewer: mehdi_amini.
tejohnson added a subscriber: llvm-commits.
mehdi_amini accepted this revision.Dec 15 2016, 11:39 AM
mehdi_amini edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Dec 15 2016, 11:39 AM
mehdi_amini added inline comments.Dec 15 2016, 11:40 AM
lib/Transforms/IPO/FunctionImport.cpp
240 ↗(On Diff #81380)

This comment was out-of-date by the way, it made me think the change was not NFC at first.

tejohnson added inline comments.Dec 15 2016, 1:04 PM
lib/Transforms/IPO/FunctionImport.cpp
240 ↗(On Diff #81380)

Good point. Removed separately in r289871 to avoid confusion.

This revision was automatically updated to reflect the committed changes.