[PATCH] D22602: [InstSimplify][InstCombine] don't crash when folding vector selects of icmp

[Sanjay Patel via llvm-commits]

spatel created this revision.
spatel added reviewers: eli.friedman, mkuper, majnemer, ibadawi.
spatel added a subscriber: llvm-commits.
Herald added a subscriber: mcrosier.

As Eli pointed out in D22537 , we have big chunks of duplicated code for similar folds in InstSimplify and InstCombine. I started on the path to refactoring that in rL276140, but it looks like it's going to be a long haul to make sure vectors work in all cases.

As Eli also noted in D22357, the code doesn't work for vector types. Not only does it not fold, it crashes. So this is hopefully a quick fix for both places before I get back to the refactoring job. 

I think we need to keep the 'getTypeSizeInBits()' call to handle aggregate types, but use getScalarSizeInBits() for vectors.

Note that I've added a scalar test for the InstCombine fold because it doesn't exist anywhere. There are likely many more of these patterns that simply have no tests. I plan to continue adding those in subsequent patches.

https://reviews.llvm.org/D22602

Files:
  lib/Analysis/InstructionSimplify.cpp
  lib/Transforms/InstCombine/InstCombineSelect.cpp
  test/Transforms/InstCombine/select.ll
  test/Transforms/InstSimplify/select.ll

Index: test/Transforms/InstSimplify/select.ll
===================================================================
--- test/Transforms/InstSimplify/select.ll
+++ test/Transforms/InstSimplify/select.ll
@@ -119,6 +119,17 @@
   ret i32 %cond
 }
 
+define <2 x i8> @test11vec(<2 x i8> %X) {
+; CHECK-LABEL: @test11vec(
+; CHECK-NEXT:    [[AND:%.*]] = and <2 x i8> %X, <i8 127, i8 127>
+; CHECK-NEXT:    ret <2 x i8> [[AND]]
+;
+  %cmp = icmp sgt <2 x i8> %X, <i8 -1, i8 -1>
+  %and = and <2 x i8> %X, <i8 127, i8 127>
+  %sel = select <2 x i1> %cmp, <2 x i8> %X, <2 x i8> %and
+  ret <2 x i8> %sel
+}
+
 define i32 @select_icmp_and_8_eq_0_or_8(i32 %x) {
 ; CHECK-LABEL: @select_icmp_and_8_eq_0_or_8(
 ; CHECK-NEXT:    [[OR:%.*]] = or i32 %x, 8
Index: test/Transforms/InstCombine/select.ll
===================================================================
--- test/Transforms/InstCombine/select.ll
+++ test/Transforms/InstCombine/select.ll
@@ -1742,3 +1742,26 @@
   %s1 = select i1 %c1, i32 %s0, i32 -1
   ret i32 %s1
 }
+
+define i32 @select_icmp_slt0_xor(i32 %x) {
+; CHECK-LABEL: @select_icmp_slt0_xor(
+; CHECK-NEXT:    [[TMP1:%.*]] = or i32 %x, -2147483648
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+  %cmp = icmp slt i32 %x, zeroinitializer
+  %xor = xor i32 %x, 2147483648
+  %x.xor = select i1 %cmp, i32 %x, i32 %xor
+  ret i32 %x.xor
+}
+
+define <2 x i32> @select_icmp_slt0_xor_vec(<2 x i32> %x) {
+; CHECK-LABEL: @select_icmp_slt0_xor_vec(
+; CHECK-NEXT:    [[TMP1:%.*]] = or <2 x i32> %x, <i32 -2147483648, i32 -2147483648>
+; CHECK-NEXT:    ret <2 x i32> [[TMP1]]
+;
+  %cmp = icmp slt <2 x i32> %x, zeroinitializer
+  %xor = xor <2 x i32> %x, <i32 2147483648, i32 2147483648>
+  %x.xor = select <2 x i1> %cmp, <2 x i32> %x, <2 x i32> %xor
+  ret <2 x i32> %x.xor
+}
+
Index: lib/Transforms/InstCombine/InstCombineSelect.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -553,8 +553,12 @@
     }
   }
 
+  // FIXME: This code is nearly duplicated in InstSimplify. Using/refactoring
+  // decomposeBitTestICmp() might help.
   {
-    unsigned BitWidth = DL.getTypeSizeInBits(TrueVal->getType());
+    unsigned BitWidth = TrueVal->getType()->isVectorTy()
+                            ? TrueVal->getType()->getScalarSizeInBits()
+                            : DL.getTypeSizeInBits(TrueVal->getType());
     APInt MinSignedValue = APInt::getSignBit(BitWidth);
     Value *X;
     const APInt *Y, *C;
Index: lib/Analysis/InstructionSimplify.cpp
===================================================================
--- lib/Analysis/InstructionSimplify.cpp
+++ lib/Analysis/InstructionSimplify.cpp
@@ -3416,7 +3416,11 @@
   if (!match(CondVal, m_ICmp(Pred, m_Value(CmpLHS), m_Value(CmpRHS))))
     return nullptr;
 
-  unsigned BitWidth = Q.DL.getTypeSizeInBits(TrueVal->getType());
+  // FIXME: This code is nearly duplicated in InstCombine. Using/refactoring
+  // decomposeBitTestICmp() might help.
+  unsigned BitWidth = TrueVal->getType()->isVectorTy()
+                          ? TrueVal->getType()->getScalarSizeInBits()
+                          : Q.DL.getTypeSizeInBits(TrueVal->getType());
   APInt MinSignedValue = APInt::getSignBit(BitWidth);
   if (ICmpInst::isEquality(Pred) && match(CmpRHS, m_Zero())) {
     Value *X;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22602.64779.patch
Type: text/x-patch
Size: 3364 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160720/9b6c80c0/attachment.bin>