[PATCH] [InstCombine] Fix wrong folding of constant comparisons involving ashr and negative values (PR21222).

Andrea Di Biagio Andrea_DiBiagio at sn.scee.net
Thu Oct 9 03:57:06 PDT 2014


Hi dexonsmith, suyog, hfinkel,

Hi,

Here is a case where the instruction combiner wrongly folds an 'icmp' instruction.
;;
define i1 @test(i32 %B) {
  %shr = ashr i32 -93, %B
  %cmp = icmp eq i32 %shr, -2
  ret i1 %cmp
}
;;

Function @test returns true when %B is equal to 6 (that is because -93 >> 6 = -2).
However, the Instruction Combiner wrongly thinks that there is no 'B' that solves equation: -93 >> B == -2.
Consequently, the entire body of function @test is wrongly folded into a `return i1 false`.

The problem is in the folding logic located in method InstCombiner::FoldICmpCstShrCst (InstCombineCompares.cpp) which wrongly computes the distance between two negative values.

With:
  AP1 < 0
  AP2 < 0
  AP2 != 0
  AP1 != 0
  AP1 > AP2

The distance between AP2 and AP1 (i.e. the distance between the highest bits set) is wrongly computed as:
  (-AP2).logBase2() - (-AP1).logBase2()

It should be computed instead taking the ones' complement of AP2 and AP1:
  (~AP2).logBase2() - (~AP1).logBase2()

This patch fixes the problem with the wrong folding of icmp.

Please let me know if ok to submit.
Thanks!
Andrea

http://reviews.llvm.org/D5700

Files:
  lib/Transforms/InstCombine/InstCombineCompares.cpp
  test/Transforms/InstCombine/icmp-shr.ll
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D5700.14646.patch
Type: text/x-patch
Size: 1021 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141009/fd52589d/attachment.bin>


More information about the llvm-commits mailing list