[llvm-commits] [llvm] r137570 - in /llvm/trunk: lib/Transforms/InstCombine/InstructionCombining.cpp test/Transforms/InstCombine/nsw.ll

[Eli Friedman]

On Sat, Aug 13, 2011 at 8:43 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> Eli Friedman wrote:
>>
>> On Sat, Aug 13, 2011 at 6:45 PM, Nick Lewycky<nicholas at mxc.ca>  wrote:
>>>
>>> Author: nicholas
>>> Date: Sat Aug 13 20:45:19 2011
>>> New Revision: 137570
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=137570&view=rev
>>> Log:
>>> Teach instcombine to preserve the nsw bit by doing an after-the-fact
>>> analysis
>>> when combining add and sub instructions. Patch by Pranav Bhandarkar!
>>
>> This appears to perform an incorrect transform for the following testcase:
>>
>> define i32 @f(i32 %x) {
>>   %y = add i32 %x, 1
>>   %z = add nsw i32 %y, 2
>>   ret i32 %z
>> }
>
> Thanks. Fixed in r137572.
>
> On a related note, I'm concerned about the reassociate pass. Consider:
>
> $ cat nsw.ll
> define i8 @test1(i8 %x) nounwind {
> entry:
>  %add = sub nsw i8 100, %x
>  %add3 = add nsw i8 %add, 80
>  ret i8 %add3
> }
> $ opt -reassociate nsw.ll -S
> ; ModuleID = 'nsw.ll'
>
> define i8 @test1(i8 %x) nounwind {
> entry:
>  %x.neg = sub i8 0, %x
>  %add3 = add nsw i8 %x.neg, -76
>  ret i8 %add3
> }

I'm not surprised it has issues...

-Eli