[PATCH] D33922: Apply summary-based dead stripping to regular LTO modules with summaries.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc marked an inline comment as done.
pcc added inline comments.


================
Comment at: llvm/lib/LTO/LTO.cpp:509
+  } else {
+    return linkRegularLTO(std::move(*ModOrErr), /*LivenessFromIndex=*/false);
+  }
----------------
tejohnson wrote:
> pcc wrote:
> > tejohnson wrote:
> > > Why not just link all of the regular LTO objects during runRegularLTO? I guess you still need to keep track of which had summaries though. How much of addRegularLTO could then be deferred until runRegularLTO (i.e. keeping track of the resolution info, like we do for ThinLTO, but delaying building the Module, computing Keep, etc)? It seems conceptually simpler to do all the linking in one place.
> > I think the first thing I tried was something along those lines, but the code turned out to be less clean in my opinion, precisely because there was more state to keep track of. I guess it's just a matter of opinion, but this seems cleaner to me, as there turned out to be a relatively clean boundary between preparing to link a module and linking it.
> Ok. But is this affected by the change being discussed in D34156, where my understanding is that all regular LTO modules will have summaries?
After that change lands all LTO modules produced by clang will have summaries, so we will normally be following the new code path that defers the link step. But we will still need to be able to read modules without summaries for backwards compatibility reasons and in order to handle bitcode created by non-clang producers.


https://reviews.llvm.org/D33922