[PATCH] D16795: WholeProgramDevirt: introduce.

[Mehdi AMINI via llvm-commits]

joker.eph added a comment.

Looking what's involved, I am not convinced that the current "bitset"/assume intrinsics structure is the most straightforward/efficient representation to communicate the information from the front-end to the analysis. It seems to complicate quite noticeably the implementation here.


================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:644
@@ +643,3 @@
+  // whole-program devirtualization and bitset lowering.
+  PM.add(createGlobalDCEPass());
+
----------------
This is the beginning of the pipeline right? When do we have unused virtual tables emitted by the front-end? I don't expect this to happen.

================
Comment at: lib/Transforms/IPO/WholeProgramDevirt.cpp:216
@@ +215,3 @@
+
+struct WholeProgramDevirt : public ModulePass {
+  static char ID;
----------------
That's not friendly to the new pass manager, can you refactor with a very thin wrapper class inheriting from ModulePass and a class for the Devirtualizer itself?

================
Comment at: lib/Transforms/IPO/WholeProgramDevirt.cpp:258
@@ +257,3 @@
+INITIALIZE_PASS_END(WholeProgramDevirt, "wholeprogramdevirt",
+                    "Whole program devirtualization", false, false)
+char WholeProgramDevirt::ID = 0;
----------------
There is a `INITIALIZE_PASS` macro that does the job if you don't need dependencies.

================
Comment at: lib/Transforms/IPO/WholeProgramDevirt.cpp:558
@@ +557,3 @@
+
+bool WholeProgramDevirt::runOnModule(Module &Mod) {
+  M = &Mod;
----------------
This function is pretty long and could probably be split.

================
Comment at: lib/Transforms/IPO/WholeProgramDevirt.cpp:602
@@ +601,3 @@
+        findLoadCallsAtConstantOffset(BitSet, Ptr, 0, CI->getArgOperand(0));
+    }
+
----------------
It seems to me that you have to reconstruct information that could have been generated by the front-end, using a representation like the intrinsic I sent by email.
This would make completely straightforward since you'd have all the uses off-hand.

================
Comment at: lib/Transforms/IPO/WholeProgramDevirt.cpp:616
@@ +615,3 @@
+  NamedMDNode *BitSetNM = M->getNamedMetadata("llvm.bitsets");
+  std::vector<VTableBits> Bits;
+  if (BitSetNM) {
----------------
Could be documented (and renamed maybe?)


http://reviews.llvm.org/D16795