[PATCH] D30016: WholeProgramDevirt: For VCP use a 32-bit ConstantInt for the byte offset.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:934
     // Rewrite each call to a load from OffsetByte/OffsetBit.
-    Constant *ByteConst = ConstantInt::get(Int64Ty, OffsetByte);
+    Constant *ByteConst = ConstantInt::get(Int32Ty, OffsetByte);
     Constant *BitConst = ConstantInt::get(Int8Ty, 1ULL << OffsetBit);
----------------
pcc wrote:
> tejohnson wrote:
> > What happens if OffsetByte is bigger than 2^32? Is that possible?
> Not without exceeding limits that are already in place. You would either need a vtable larger than 2^31 bytes (i.e. a vtable containing more than 2^29 (32-bit) or 2^28 (64-bit) virtual functions, which not only far exceeds the recommended minimum limits of the C++ standard, but by itself exceeds the limits of the small memory model) or enough VCP eligible virtual calls to cause the size of the vtable's constant region to extend beyond 2^31 (that would require at least 2^28 such vcalls, and most likely the instructions making up those vcalls would by themselves push the size beyond the small memory model limit).
> 
> And of course, neither scenario is very realistic.
I have seen a couple of apps that exceeded the small memory model, although it seems unlikely that we would have something with a vtable or virtual calls by themselves to hit this limit. Unlikely, but might be nice to have a reasonable failure mode if we need to go to to a larger memory model...


https://reviews.llvm.org/D30016