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

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 14 13:23:09 PST 2017


pcc added inline comments.


================
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(),
----------------
tejohnson wrote:
> Why are the ZExt here and in below function now required?
This one is needed because there is now the possibility of a mismatch between the return type of the function and the type of `Cmp` (always i1 before). I've added a test showing that we handle that case.

The one in tryVirtualConstProp is unnecessary, I've removed it.


================
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
----------------
tejohnson wrote:
> Why are these changes useful/necessary? Is there less coverage with the return type being dropped to i64?
This change was to cover specifically the rejection due to the wide argument type. Good catch for the return type, I've re-added coverage for that.


================
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
----------------
tejohnson wrote:
> Comment about why we can do optimization (function name still just says "bad_arg_type", which implies not being able to do it).
Added a comment to the top of the file.


https://reviews.llvm.org/D29744





More information about the llvm-commits mailing list