[llvm] r209746 - InstCombine: Improvement to check if signed addition overflows.

suyog sarda sardask01 at gmail.com
Wed Jun 4 10:27:01 PDT 2014


Thanks Rafael.

I observed the test cases modified. Will take care of fine tuning test
cases in future. :)

Thanks a lot.


On Wed, Jun 4, 2014 at 9:19 PM, Rafael EspĂ­ndola <rafael.espindola at gmail.com
> wrote:

> r210186. I just changed a few things before commit:
>
> * checkRippleForAdd was using only two arguments.
> * It could use an early return.
> * Reduce the tests to use the fact that nsw is now set for any add
> that passes WillNotOverflowSignedAdd, so we don't need any sign
> extensions to complicate the test.
>
>
> On 4 June 2014 02:21, Suyog Kamal Sarda <suyog.sarda at samsung.com> wrote:
> > Hi Rafael,
> >
> > Attached updated patch with minor changes - starting letter small of
> helper function + passing function parameters by const reference.
> >
> > To keep the helper function generic, i created a local copy of
> Op0KnownZero as we require to clear MSB bit.
> > Please see if this looks good to you.
> >
> > Thanks,
> > Suyog Sarda
> >
> > ------- Original Message -------
> > Sender : Rafael EspĂ­ndola<rafael.espindola at gmail.com>
> > Date : Jun 03, 2014 23:53 (GMT+09:00)
> > Title : Re: Re: Re: [llvm] r209746 - InstCombine: Improvement to check
> if signed addition overflows.
> >
> > I updated my patch with the test cases. Can you please check if this is
> correct?
> > If it looks good to you, can you please commit it on my behalf?
> >
> > My patch also handles the case, where  one of the operand has only 1 bit
> that is not known to
> >
> > be zero, but it is also not know to be one.
> > ((~RHSKnownZero).countpopulation == 1) will be able to determine if only
> one of the bit is unkown (it may be 1 or 0).
> >
> > As far as the test case :
> >
> > define i32 @foo(i32 %x, i32 %y) {
> >   %v1 = and i32 %y, 1
> >   %v2 = add i32 %x, %v1
> >   ret i32 %v2
> > }
> >
> > We know that only one of the bit is unknown of one of the operand, it
> may be 1 or 0.
> > But for the other operand, we do not know if it has 0 at any higher bit
> position. Hence we do not add 'nsw' in this case.
> >
> > Added this test case as well + test case to handle sext  i1(bitwidth =
> 1).
> >
> > Awaiting for your review.
> >
> >
> > The check for a bitwidth of one seems redundant. The updated computation
> of Op1OnePosition handles that case too, no?
> >
> >
> > It looks like the tests can be simplified a bit further. They are still
> using numbered instead of named instructions for example.
> >
> >
> > Please make sure the positive tests are cases that currently fail, so
> they demonstrate the new optimization capabilities. For example, we already
> add a nsw to  @ripple_nsw because of the current logic for handling both
> operands having more than one sign bit.
> >
> >
> > Cheers,
> > Rafael
>



-- 
With regards,
Suyog Sarda
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140604/d6845948/attachment.html>


More information about the llvm-commits mailing list