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

Rafael EspĂ­ndola rafael.espindola at gmail.com
Mon Jun 2 07:02:38 PDT 2014


Something wrong with the patch:

patch -p0 < ~/Downloads/AddOverFlow.patch
patching file lib/Transforms/InstCombine/InstCombineAddSub.cpp
patch: **** malformed patch at line 84: Index:
test/Transforms/InstCombine/AddOverFlow.ll

+  %1 = sext i16 %x to i32

Please run "opt -instnamer" (or manually name the instructions), that make
the test a lot easier to read.





On 2 June 2014 08:55, Suyog Kamal Sarda <suyog.sarda at samsung.com> wrote:

>  Hi Rafael,
>
>
>
> I found the bug. Basically, this was giving problem for operands having
> bitwidth 1.
>
> It was always returning true for 'WillNotOverFlowSignedAdd' and hence the
> test case :
>
>
>
>
>
>
>
>
> *define i32 @foo(i32 %x, i1 %y) {%v1 = sext i1 %y to i32%v2 = add i32 %v1,
> -1ret i32 %v2}*
>
> was getting reduced to
>
>
>
>
>
> *define i32 @foo(i32 %x, i1 %y) {%addconv = xor i1 %y, true%v2 = sext i1
> %addconv to i32ret i32 %v2**}*
>
>
>
> Because -1 is represented as 1 in 1 bit representation, and it was always
> returning true for NotSignedOverFlow,
>
> the carry bit was getting ignored and the add was getting converted to xor
> operations (since 'xor' = 'add neglecting carry bit' )
>
> causing error.
>
>
>
> I have handled this condition in the updated patch -> return true only if
> one of the operand has 0 bit for BitWidth 1.
>
> Added the above test case as well.
>
>
>
> Attaching updated patch. Please help in reviewing it.
>
>
>
> Thanks !
>
>
> Regards,
>
> Suyog Sarda
>
>
>
> ------- *Original Message* -------
>
> *Sender* : Rafael EspĂ­ndola<rafael.espindola at gmail.com>
>
> *Date* : May 31, 2014 04:06 (GMT+09:00)
>
> *Title* : Re: [llvm] r209746 - InstCombine: Improvement to check if
> signed addition overflows.
>
>
> > Strangely though, removing the symmetric call gives good result of
>
> > test-func.ll you had provided (I donno how but it works, even though the
> > second call would be dead).
>
> Well, we have to understand what is wrong with it.
>
> The testcase reduces to just
>
> define i32 @foo(i32 %x, i1 %y) {
>   %v1 = sext i1 %y to i32
>   %v2 = add i32 %v1, -1
>   ret i32 %v2
> }
>
> which with the patch gets converted to
>
> define i32 @foo(i32 %x, i1 %y) {
>   %addconv = xor i1 %y, true
>   %v2 = sext i1 %addconv to i32
>   ret i32 %v2
> }
>
> Can you step over instcombine and see what invalid transformation is being
> done?
>
> Cheers,
> Rafael
>
>
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140602/1407f974/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 201406021826324_QKNMBDIF.gif
Type: image/gif
Size: 13168 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140602/1407f974/attachment.gif>


More information about the llvm-commits mailing list