Consider the attached bytecode. This is just -O0 compiled bytecode from gcc testsuite. Running "opt -instcombine" will lead to "eq" comparison turn to "slt", which is definitely wrong.
Created attachment 701 [details] Bytecode
I think that this is trying to simplify (x sdiv 12) != -6
Here's a reduced testcase: define i1 @test(i32 %tmp6) { %tmp7 = sdiv i32 %tmp6, 12 ; <i32> [#uses=1] icmp ne i32 %tmp7, -6 ; <i1>:0 [#uses=1] ret i1 %0 } instcombine compiles it to: define i1 @test(i32 %tmp6) { icmp sgt i32 %tmp6, -73 ; <i1>:0 [#uses=1] ret i1 %0 } which is wrong for an input of -70. -Chris
This appears to be fallout from the signless types change. The problem is AddWithOverflow in Instcombine. It is not calculating overflow correctly if the operands are signed. The getZExtValue() < getZExtValue() comparison needs to do a signed comparison if the input operands are signed (requiring a bool to be passed in). Reid, plz investigate. -Chris
Mine.
Making the AddWithOverflow change doesn't fix the problem.
Turns out there were two bugs. The AddWithOverflow needed to be APIntified and to deal with the sign properly. Also, in the case of: %Y = sdiv iN %X, C1 icmp pred iN %Y, -C2 there was an off-by-one error in the high bound for the range test that is produced in the replacement. This patch was applied: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070319/046072.html