[PATCH] D29744: WholeProgramDevirt: Change internal vcall data structures to match summary.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:262
 
+struct CallSiteInfo {
+  std::vector<VirtualCallSite> CallSites;
----------------
Document the new structures


================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:268
+  CallSiteInfo CSInfo;
+  std::map<std::vector<uint64_t>, CallSiteInfo> ConstCSInfo;
+
----------------
Please document what this map is holding (vs the single CSInfo above). I believe I deduced it from code below, but better to document so one doesn't need to see how these are accessed to understand.


================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:582
                                 Call.VTable, OneAddr);
+      Cmp = B.CreateZExt(Cmp, Call.CS->getType());
       Call.replaceAndErase("unique-ret-val", TargetsForSlot[0].Fn->getName(),
----------------
Why are the ZExt here and in below function now required?


================
Comment at: llvm/test/Transforms/WholeProgramDevirt/vcp-too-wide-ints.ll:10
+define i64 @vf1(i8* %this, i128 %arg) readnone {
+  %argtrunc = trunc i128 %arg to i64
+  ret i64 %argtrunc
----------------
Why are these changes useful/necessary? Is there less coverage with the return type being dropped to i64?


================
Comment at: llvm/test/Transforms/WholeProgramDevirt/vcp-type-mismatch.ll:28
   %result = call i32 %fptr_casted(i8* %obj, i64 1)
+  ; CHECK: ret i32 1
   ret i32 %result
----------------
Comment about why we can do optimization (function name still just says "bad_arg_type", which implies not being able to do it).


================
Comment at: llvm/test/Transforms/WholeProgramDevirt/vcp-type-mismatch.ll:58
   %result = call i64 %fptr_casted(i8* %obj, i32 1)
+  ; CHECK: ret i64 1
   ret i64 %result
----------------
ditto


https://reviews.llvm.org/D29744