[PATCH] D33615: Move summary dead stripping before regular LTO and record results in the combined summary

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.





================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:536
 
+  /// Indicates that summary-based GlobalValue GC has run, and values with
+  /// GVFlags::Live==false are really dead. Otherwise, all values must be
----------------
Do not use "GC": let's not switch terminology between multiple places I think we used "Dead-Stripping" till now.


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:561
+    return !WithGlobalValueGc || GVS->isLive();
+  }
+
----------------
If you want to go this route of making the index explicitly modal, then `GlobalValueSummary::isLive()` should be private and the `ModuleSummaryIndex` can be friended.


================
Comment at: lib/LTO/LTO.cpp:636
+  computeDeadSymbols(ThinLTO.CombinedIndex, GUIDPreservedSymbols);
+
   // Save the status of having a regularLTO combined module, as
----------------
We reached a point where this patch is complex enough that I rather see it split between the change to make the `live` flags in the summary replacing all the sets (NFC changes) and actually moving the place where it is performed.


Repository:
  rL LLVM

https://reviews.llvm.org/D33615