[PATCH] D16795: WholeProgramDevirt: introduce.

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 5 23:19:45 PST 2016

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?)


More information about the llvm-commits mailing list