Another InstCombine issue

[asl]

Bugzilla Link	1248
Resolution	FIXED
Resolved on	Feb 22, 2010 12:44
Version	trunk
OS	All
Blocks	#1538
Attachments	Bytecode
CC	@lattner

Extended Description

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.

[lattner]

I think that this is trying to simplify (x sdiv 12) != -6

[lattner]

Here's a reduced testcase:
define i1 @​test(i32 %tmp6) {
%tmp7 = sdiv i32 %tmp6, 12 ; [#uses=1]
icmp ne i32 %tmp7, -6 ; :0 [#uses=1]
ret i1 %0
}

instcombine compiles it to:

define i1 @​test(i32 %tmp6) {
icmp sgt i32 %tmp6, -73 ; :0 [#uses=1]
ret i1 %0
}

which is wrong for an input of -70.

-Chris

[lattner]

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

[llvmbot]

Mine.

[llvmbot]

Making the AddWithOverflow change doesn't fix the problem.

[llvmbot]

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