[PATCH] D26581: Make WholeProgramDevirt understand ConstStruct vtables.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lib/Transforms/IPO/WholeProgramDevirt.cpp:392-416
+    const DataLayout &DL = M.getDataLayout();
+    const Constant *I = TM.Bits->GV->getInitializer();
+    const uint64_t GlobalSlotOffset = TM.Offset + ByteOffset;
+    unsigned Op;
 
-    uint64_t ElemSize =
-        M.getDataLayout().getTypeAllocSize(VTableTy->getElementType());
-    uint64_t GlobalSlotOffset = TM.Offset + ByteOffset;
-    if (GlobalSlotOffset % ElemSize != 0)
-      return false;
+    if (auto *C = dyn_cast<ConstantStruct>(I)) {
+      const StructLayout *SL = DL.getStructLayout(C->getType());
----------------
All this code could be factored out into a separate function.


================
Comment at: lib/Transforms/IPO/WholeProgramDevirt.cpp:403
+
+      Op = SL->getElementContainingOffset(GlobalSlotOffset);
+    } else if (auto *C = dyn_cast<ConstantArray>(I)) {
----------------
I think you also want to check that `SL->getElementOffset(Op) == GlobalSlotOffset`.

Please add negative tests for that case and the out-of-bounds case you have above.


================
Comment at: test/Transforms/WholeProgramDevirt/non-array-vtable.ll:30
 
+; CHECK: define void @call2
+define void @call2(i8* %obj) {
----------------
Please put your test in a separate file and rename this to something like `non-aggregate-vtable.ll`.


https://reviews.llvm.org/D26581