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

Rafael Espíndola rafael.espindola at gmail.com
Mon Jun 2 07:47:40 PDT 2014


BTW, I was curious why the tests looked more complicated than needed. It
turns out we were not just adding nsw based on WillNotOverflowSignedAdd. I
changed that in r210029. It should allow you to simplify the tests further.


On 2 June 2014 10:02, Rafael Espíndola <rafael.espindola at gmail.com> wrote:

>
> 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/1c0969a3/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/1c0969a3/attachment.gif>


More information about the llvm-commits mailing list