[PATCH] InstCombine: constant comparison involving ashr is wrongly simplified (PR20945).

[Andrea Di Biagio]

Hi suyog, dexonsmith, hfinkel,

Example:

;;;;
define i32 @negative_constants(i32 %B) {
entry:
  %shr = ashr i32 -9, %B
  %cmp = icmp ne i32 %shr, -5
  br i1 %cmp, label %if.then, label %return

if.then:
  br label %return

return:
  %retval = phi i32 [ 0, %if.then ], [ 42, %entry ]
  ret i32 %retval
}
;;;

The instruction combiner wrongly thinks that statement 'ashr i32 -9, %B' can never evaluate to -5. Therefore, it wrongly assumes that %cmp would always be 'true' and the branch to 'if.then' would always be taken.

In this reproducible, if %B is equal to 1, then %cmp is 'false' (since %shr would be equal to -5).
Therefore, it is not safe to fold %cmp to 'true'. What instead should be done is converting the comparison between %shr and -5 into a comparison between %B and 1. This would allow us to get rid of the arithmetic shift (and eventually simplify the CFG folding the compare-and-branch into a single select of either 0 or 42).

This is a regression introduced by revision 213678. This bug seems to affect only the case where constants are both negative values.

Please let me know if ok to submit.

Thanks,
Andrea

http://reviews.llvm.org/D5356

Files:
  lib/Transforms/InstCombine/InstCombineCompares.cpp
  test/Transforms/InstCombine/icmp-shr.ll
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D5356.13712.patch
Type: text/x-patch
Size: 2788 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140915/285862bd/attachment.bin>