[PATCH] D71913: [LTO/WPD] Enable aggressive WPD under LTO option

[Eugene Leviant via Phabricator via cfe-commits]

evgeny777 added inline comments.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:559
+  if (!CodeGenOpts.ThinLTOIndexFile.empty())
+    MPM.add(createLowerTypeTestsPass(/*ExportSummary=*/nullptr,
+                                     /*ImportSummary=*/nullptr,
----------------
Test case?


================
Comment at: clang/test/CodeGenCXX/lto-visibility-inference.cpp:73
   c1->f();
-  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C2"
+  // ITANIUM: type.test{{.*}}!"_ZTS2C2"
   // MS: type.test{{.*}}!"?AUC2@@"
----------------
What caused this and other changes in this file?


================
Comment at: llvm/include/llvm/Transforms/IPO.h:245
+                                     const ModuleSummaryIndex *ImportSummary,
+                                     bool StripAll = false);
 
----------------
s/StripAll/DropTypeTests/g ?


================
Comment at: llvm/lib/LTO/LTOCodeGenerator.cpp:550
+  // pipeline run below.
+  updateVCallVisibilityInModule(*MergedModule);
+
----------------
I'd rather use
```
updateVCallVisibilityInModule(*MergedModule,  /* WholeProgramVisibilityEnabledInLTO */ false)
```
and remove default value for second parameter


================
Comment at: llvm/lib/LTO/ThinLTOCodeGenerator.cpp:976
+  // via the internal option. Must be done before WPD below.
+  updateVCallVisibilityInIndex(*Index);
+
----------------
ditto


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1775
+    if (TypeTestFunc) {
+      for (auto UI = TypeTestFunc->use_begin(), UE = TypeTestFunc->use_end();
+           UI != UE;) {
----------------
Fold identical code blocks


================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:145
+/// when enabled via the linker.
+cl::opt<bool> DisableWholeProgramVisibility(
+    "disable-whole-program-visibility", cl::init(false), cl::Hidden,
----------------
Is this tested?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71913