[PATCH] D55060: [ThinLTO] Look through aliases in cache key calculations

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 3 11:34:38 PST 2018


tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lib/LTO/LTO.cpp:230
     for (auto &ImpF : ImpM.second)
       AddUsedThings(Index.findSummaryInModule(ImpF, ImpM.first()));
 
----------------
george.burgess.iv wrote:
> tejohnson wrote:
> > Presumably this is where we were missing the base object, because we may have the alias in the import list but not its base object (but we will actually import the alias as a copy of the base object). My suggestion would be to check if the summary is an alias here, and if so call AddUsedThings a second time on the base object summary.
> SGTM.
> 
> The `_or_null` in my new patch was out of caution rather than actual test failures. Please let me know if I should be able to assume `S != nullptr`, and I'm happy to drop that. :)
I would have thought we should get a non-null S, but the existing code handles it (the early return in AddUsedThings), so it is ok with me to keep the _or_null here.


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

https://reviews.llvm.org/D55060





More information about the llvm-commits mailing list