[PATCH] InstCombine Check if Signed Addition Overflows

[suyog sarda]

On Tue, May 27, 2014 at 7:29 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> Sorry for the delay, I was stuck in the GlobalAlias patches.
>
> Please be sure to address Jingyue Wu comment too.
>
>
Oops !! Got the point. Thanks Jingyue Wu for pointing out. Now i am doing
this by
((~RHSKnownZero).countpopulation == 1) which combines (
RHSKnownOne.countpopulation == 1 && RHSKnownZero.countpopulation ==
BitWidth-1 ). This will ensure that only one bit is 1 and all other bits
are 0.
Also added a test case related to this e.g (x & ~4) + (x | 2) -> this we
are not sure if overflow doesn't occur
as we are not sure of other bits in (x | 2) -> so 'add' will not convert to
'add nsw'.


> +    APInt LHSKnownZero(BitWidth, 0, 1);


> Since the value is 0, there is no difference in passing isSigned=true.
> If you think that is better to be explicit, please write it as
>
> APInt LHSKnownZero(BitWidth, 0, /*isSigned*/ true);
>

Made this change.


>
> +    // TODO: Handle for Vectors.
>
> The comment is more for the entire function, no just this particular
> case. Please move it to the top.
>
>
Done !


> +  if (RHSKnownOne.countPopulation() == 1)
>
> Jingyue Wu comment....
>

Implemented.


>
> Please don't duplicate the code to handle the symmetry. You can
> probably use a small static helper function and write
>
> if (helper(LHSKnowZero, LHSKnowOne, RHSKnownZero, RHSKnownOne))
>   return true;
>
> if (helper(RHSKnownZero, RHSKnownOne, LHSKnowZero, LHSKnowOne))
>   return true;
>
>
Added a static helper function to do this.


> +      int LHSZeroPosition = BitWidth - LHSKnownZero.countLeadingZeros() -
> 1;
>
> Please add a FIXME about this missing the case where LHS has a zero
> before the 1 in the RHS, but also has one after.
>
>
Added this FIXME.


> Cheers,
> Rafael
>

Thanks a lot Rafael, Jingyue Wu for helping out in this implementation.
Attaching updated patch.
Please see if this looks good.

-- 
With regards,
Suyog Sarda
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140527/2ae76308/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: AddOverFlow.patch
Type: text/x-patch
Size: 5098 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140527/2ae76308/attachment.bin>