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

[suyog sarda]

Hi Rafael,

I compared the two files 'test-func.ll' and 'test-bad.ll' . I found 1
difference :

test-func.ll


*land.lhs.true47:                                  ; preds = %if.end38
%dec = sext i1 %tobool to i32  %dec.call27 = add i32 %dec, -1*


test-bad.ll


*land.lhs.true47:                                  ; preds = %if.end38
%addconv = xor i1 %tobool, true  %dec.call27 = sext i1 %addconv to i32*

In the patch, i removed the symmetric call -





*// Check if carry bit of addition will not cause overflow.    if
(CheckRippleForAdd(LHSKnownZero, LHSKnownOne, RHSKnownZero,
RHSKnownOne))      return true;    if (CheckRippleForAdd(RHSKnownZero,
RHSKnownOne, LHSKnownZero, LHSKnownOne))      return true;*  --> Removed
this call

After removing this last call, when i run opt -instcombine on test-func.ll,
the output is same as test-func.ll (seems the issue is resolved atleast
from output.)

We can remove the symmetric call because we are sure that one of the
operand is constant (as we are determining if we know each bit of one of
the operand - one bit set to 1 and all other to 0), and constants are
always arranged on RHS, despite their position in original IR. So the last
call is not required.

The updated patch is working fine for the test-func.ll file now. Can we
check if the patch with last call removed doesn't break the sanitizer and
arm built bot? I do not know how to run tests for sanitizer and arm built
bot. Can someone please run the sanitizer and arm built bot with attached
patch :)

Attaching updated patch.


On Fri, May 30, 2014 at 1:25 AM, Renato Golin <renato.golin at linaro.org>
wrote:

> On 29 May 2014 15:41, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
> > OK, I reduced this to llvm::SelectionDAGISel::MorphNode being
> miscompiled.
>
> Hi Rafael,
>
> Just FYI, this also broke the self-hosting ARM bot (but the previous
> failure may have masked the bot email):
>
> http://lab.llvm.org:8011/builders/clang-native-arm-cortex-a15-self-host
>
> It may be good to have a look on that build after your fix to make
> sure all issues have gone away. :)
>
> cheers,
> --renato
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



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