[llvm] r271908 - [InstCombine] limit icmp transform to ConstantInt (PR28011)

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Mon Jun  6 11:56:57 2016
New Revision: 271908

URL: http://llvm.org/viewvc/llvm-project?rev=271908&view=rev
Log:
[InstCombine] limit icmp transform to ConstantInt (PR28011)

In r271810 ( http://reviews.llvm.org/rL271810 ), I loosened the check
above this to work for any Constant rather than ConstantInt. AFAICT, 
that part makes sense if we can determine that the shrunken/extended 
constant remained equal. But it doesn't make sense for this later 
transform where we assume that the constant DID change. 

This could assert for a ConstantExpr:
https://llvm.org/bugs/show_bug.cgi?id=28011

And it could be wrong for a vector as shown in the added regression test.

Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
    llvm/trunk/test/Transforms/InstCombine/2004-11-27-SetCCForCastLargerAndConstant.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=271908&r1=271907&r2=271908&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Mon Jun  6 11:56:57 2016
@@ -2459,12 +2459,14 @@ Instruction *InstCombiner::visitICmpInst
     return new ICmpInst(ICmp.getUnsignedPredicate(), LHSCIOp, Res1);
   }
 
-  // The re-extended constant changed so the constant cannot be represented
-  // in the shorter type. Consequently, we cannot emit a simple comparison.
+  // The re-extended constant changed, partly changed (in the case of a vector),
+  // or could not be determined to be equal (in the case of a constant
+  // expression), so the constant cannot be represented in the shorter type.
+  // Consequently, we cannot emit a simple comparison.
   // All the cases that fold to true or false will have already been handled
   // by SimplifyICmpInst, so only deal with the tricky case.
 
-  if (isSignedCmp || !isSignedExt)
+  if (isSignedCmp || !isSignedExt || !isa<ConstantInt>(C))
     return nullptr;
 
   // Evaluate the comparison for LT (we invert for GT below). LE and GE cases

Modified: llvm/trunk/test/Transforms/InstCombine/2004-11-27-SetCCForCastLargerAndConstant.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/2004-11-27-SetCCForCastLargerAndConstant.ll?rev=271908&r1=271907&r2=271908&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/2004-11-27-SetCCForCastLargerAndConstant.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/2004-11-27-SetCCForCastLargerAndConstant.ll Mon Jun  6 11:56:57 2016
@@ -22,6 +22,34 @@ define i1 @lt_signed_to_large_unsigned(i
   ret i1 %C
 }
 
+; PR28011 - https://llvm.org/bugs/show_bug.cgi?id=28011
+; The above transform only applies to scalar integers; it shouldn't be attempted for constant expressions or vectors.
+
+ at a = common global i32** null
+ at b = common global [1 x i32] zeroinitializer
+
+define i1 @PR28011(i16 %a) {
+; CHECK-LABEL: @PR28011(
+; CHECK-NEXT:    [[CONV:%.*]] = sext i16 %a to i32
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32 [[CONV]], or (i32 zext (i1 icmp ne (i32*** bitcast ([1 x i32]* @b to i32***), i32*** @a) to i32), i32 1)
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %conv = sext i16 %a to i32
+  %cmp = icmp ne i32 %conv, or (i32 zext (i1 icmp ne (i32*** bitcast ([1 x i32]* @b to i32***), i32*** @a) to i32), i32 1)
+  ret i1 %cmp
+}
+
+define <2 x i1> @lt_signed_to_large_unsigned_vec(<2 x i8> %SB) {
+; CHECK-LABEL: @lt_signed_to_large_unsigned_vec(
+; CHECK-NEXT:    [[Y:%.*]] = sext <2 x i8> %SB to <2 x i32>
+; CHECK-NEXT:    [[C:%.*]] = icmp ult <2 x i32> [[Y]], <i32 1024, i32 2>
+; CHECK-NEXT:    ret <2 x i1> [[C]]
+;
+  %Y = sext <2 x i8> %SB to <2 x i32>
+  %C = icmp ult <2 x i32> %Y, <i32 1024, i32 2>
+  ret <2 x i1> %C
+}
+
 define i1 @lt_signed_to_large_signed(i8 %SB) {
 ; CHECK-LABEL: @lt_signed_to_large_signed(
 ; CHECK-NEXT:    ret i1 true