[PATCH] D73724: [ThinLTO] Disable "Always import constants" due to compile time issues

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

I found the reason for the rest of the increase. Thankfully it is another easy to fix inefficiency. If you look at where we mark the refs as exported in computeImportForReferencedGlobals, it is marking each *copy* of the ref as exported:

  for (const auto &VI : RefSummary->refs())
    for (const auto &RefFn : VI.getSummaryList())
      MarkExported(VI, RefFn.get());

For our huge applications, with thousands of files, that means that for some commonly used linkonce_odr refs we are iterating significant amounts.

We actually only need to mark the copy in the same module as RefSummary (the copy being imported) as exported. I.e., see the equivalent code in computeImportForFunction:

  for (auto &Ref : ResolvedCalleeSummary->refs())
    ExportList.insert(Ref);

The reason is that we only need to know which symbols that were not previously exported (i.e. were local, or otherwise previously referenced outside the defining module) are now exported due to this import (so that we don't internalize, or promote if needed). E.g. if you look at the caller in runThinLTO, we've already identified which globals were live and exported (referenced in another module).

When I change the code in computeImportForReferencedGlobals to only mark the copy in RefSummary's module as exported, the time drops to within a couple seconds (2.7%) of the version with this not enabled.

I'll work on a cleaned up patch with these efficiency fixes and send for review. After that goes in and I do a bunch more testing with some of our large apps, we can probably turn this back on by default.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73724/new/

https://reviews.llvm.org/D73724