[PATCH] D134320: [llvm] Teach whole program devirtualization about relative vtables

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Sorry for the slow review. Thanks for adding support for index-only WPD, and adding a test for both that and the split-module (hybrid) LTO case! I have a few questions below.



================
Comment at: llvm/lib/Analysis/ModuleSummaryAnalysis.cpp:571
+    }
+  } else if (const auto *CE = dyn_cast<ConstantExpr>(stripCasts(I))) {
+    // If this constant can be reduced to the offset between a function and the
----------------
Do we also need to look through DSOLocalEquivalent in this function, as is done in getPointerAtOffset, or why not?


================
Comment at: llvm/lib/Analysis/ModuleSummaryAnalysis.cpp:574
+    // original vtable (`Fn - OrigGV`), then we can still find the function if
+    // we know the function.
+    if (CE->getOpcode() == Instruction::Sub) {
----------------
I don't completely understand what is meant by "find the function if we know the function".


================
Comment at: llvm/lib/Analysis/ModuleSummaryAnalysis.cpp:579
+      if (IsConstantOffsetFromGlobal(CE->getOperand(0), LHS, LHSOffset, DL) &&
+          IsConstantOffsetFromGlobal(CE->getOperand(1), RHS, RHSOffset, DL) &&
+          RHS == &OrigGV) {
----------------
Do we need to do any checking or handling of LHSOffset and RHSOffset?


================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1386
       // Jump tables are only profitable if the retpoline mitigation is enabled.
+      if (!CB.getParent()) {
+        // When finding devirtualizable calls, it's possible to find the same
----------------
What provoked needing this change, and do we (or why not) need a similar check for all the other WPD optimizations that process the CallSites?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134320