[PATCH] D35003: [MemoryBuiltins] Allow truncation in visitAllocaInst()

Mikael Holmén via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 10 00:06:09 PDT 2017


uabelho added inline comments.


================
Comment at: lib/Analysis/MemoryBuiltins.cpp:518
   if (const ConstantInt *C = dyn_cast<ConstantInt>(ArraySize)) {
-    Size *= C->getValue().zextOrSelf(IntTyBits);
+    Size *= C->getValue().zextOrTrunc(IntTyBits);
     return std::make_pair(align(Size, I.getAlignment()), Zero);
----------------
uabelho wrote:
> george.burgess.iv wrote:
> > uabelho wrote:
> > > george.burgess.iv wrote:
> > > > Addressing your comment: I realize that this is an existing inconsistency, but we try to return `unknown()` if an overflow happens (e.g. on line 597). While we're in the area, can we make this test for overflow too, please?
> > > So you want me to detect if overflow actually happens and if so return unknown(), and add a testcase for the overflow case as well?
> > > 
> > > If so, yes I'll try.
> > Yes, please.
> > 
> > I'm not 100% sure if testing this will be straightforward, though. If it seems nontrivial, just let me know. Since it's my nit, I'm happy to try to make a test for overflow and commit it after this lands. :)
> Ok I will!
> 
> Yes the testing I don't know since I think the test case I have already is pretty non-straightforward.. The i64 is being converted to a [2 x i32] and I think it's that "2" that I will truncate.
> 
> So for overflow to happen with just small modifications of my existing test I'll need my %i64_t to be something huge. I'll give it a try though.
Added overflow checks. Something like this is what you had in mind?

For the testcase, if I change
 %i64_t = type i64
to
 %i64_t = type i6488888
then the overflow checks trigger, but the end result from instcombine is still the same
as without overflow checks so I don't have any nice output change to check for.

If you know this code then I suppose you'll be able to make a testcase for it. 


https://reviews.llvm.org/D35003





More information about the llvm-commits mailing list