[PATCH] D123803: [WIP][llvm] A Unified LTO Bitcode Frontend

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/LTO/LTO.cpp:739
+  if (LTOMode == LTOK_UnifiedRegular)
+    if (NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions"))
+      M.eraseNamedMetadata(CfiFunctionsMD);
----------------
Why is this needed?


================
Comment at: llvm/lib/LTO/LTO.cpp:1128
+  if (LTOMode == LTOKind::LTOK_UnifiedRegular)
+    EnableLTOInternalization = true;
+
----------------
Why is this needed?


================
Comment at: llvm/lib/LTO/LTO.cpp:1148
+            || GV->hasAppendingLinkage()))
+        continue;
+
----------------
Why is this needed?


================
Comment at: llvm/lib/LTO/LTO.cpp:1547
+  if (Conf.UnifiedLTO)
+    Conf.PTO.CallGraphProfile = false;
+
----------------
Why is this needed?


================
Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:212
     PerformThinLTO = EnablePerformThinLTO;
+    UnifiedLTO = false;
     DivergentTarget = false;
----------------
Rather than adding the many checks in the file below, can the Perform* and PrepareFor* options just be initialized differently under the UnifiedLTO mode?


================
Comment at: llvm/lib/Transforms/Utils/ModuleUtils.cpp:225
 
+  // When UnifiedLTO is enabled, we always need to generate split modules which
+  // requires a ModuleID. Using more types of metadata to generate this hash
----------------
As noted elsewhere, I'd prefer the UnifiedLTO not imply splitting. But regardless, this seems like a useful change whenever LTO unit is enabled - why not always compute the module hash this way?


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

https://reviews.llvm.org/D123803