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

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 30 21:47:20 PST 2020


tejohnson added a comment.

I spent some time looking at one of the big increases from this change today, and found that there is a fairly straightforward fix for most of the increase. The problem is that in computeImportForReferencedGlobals, whenever a global variable is imported, we walk its set of references and mark them all as exported - every single time the global variable is imported. We only need to mark its refs as exported the first time it is exported into any other module. This patch didn't introduce the problem, but it was made much worse as we are far more likely to import global variables containing refs.

We can easily skip doing the exporting again if the ImportList insert return value indicates that the insert wasn't done because it was already in the list. But this only avoids the problem when we attempt to import the global variable into the same module multiple times. In order to see if it has been exported from anywhere, we need additional info. It isn't sufficient to check whether the ExportList insert of the variable returns that it was not inserted because it was already there. This is because we don't know whether it was previously marked exported because it itself was imported somewhere (in which case we already exported its refs), or because some other value that referenced the variable was imported and marked it exported (in which case we haven't yet marked its own refs exported). In order to do that we can either keep another set or augment the existing ExportList to keep some additional info. I prototyped a fix using a separate set to mark those global variable defs that were exported. The thin link time went down significantly with constant variable importing enabled.

Unfortunately, even with the above fix, with the importing of constant variables we are still taking about 47% more time in the thin link. The export and import lists will be longer, and I'm not sure if it is just a result of processing those longer lists, or whether there is some other issue. I'll need to do some additional measurements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73724





More information about the llvm-commits mailing list