[PATCH] D31189: [InstCombine] Avoid incorrect folding of select into phi nodes when incoming element is a vector type

[Anna Thomas via Phabricator via llvm-commits]

anna created this revision.

We are incorrectly folding selects into phi nodes when the incoming value of a phi 
node is a constant vector. This optimization is done in `FoldOpIntoPhi` when the
select condition is a phi node with constant incoming values.
Without the fix, we always choose the `true` part
of the select to fold into the phi node, irrespective of the constant vector
elements.
With this fix, we will correctly choose the element from the vector based on the 
select vector operand (see added test cases).


https://reviews.llvm.org/D31189

Files:
  lib/Transforms/InstCombine/InstructionCombining.cpp
  test/Transforms/InstCombine/phi-select-constexpr.ll


Index: test/Transforms/InstCombine/phi-select-constexpr.ll
===================================================================
--- test/Transforms/InstCombine/phi-select-constexpr.ll
+++ test/Transforms/InstCombine/phi-select-constexpr.ll
@@ -9,11 +9,49 @@
 delay:
   br label %final
 
+; CHECK-LABEL: @foo
 ; CHECK-LABEL: final:
 ; CHECK: phi i32 [ 1, %entry ], [ select (i1 icmp eq (i32* @A, i32* @B), i32 2, i32 1), %delay ]
 final:
   %use2 = phi i1 [ false, %entry ], [ icmp eq (i32* @A, i32* @B), %delay ]
   %value = select i1 %use2, i32 2, i32 1
   ret i32 %value
 }
 
+
+; test folding of select into phi for vectors.
+define <4 x i64> @vec1(i1 %which) {
+entry:
+  br i1 %which, label %final, label %delay
+
+delay:
+ br label %final
+
+final:
+; CHECK-LABEL: @vec1
+; CHECK-LABEL: final:
+; CHECK: %phinode = phi <4 x i64> [ zeroinitializer, %entry ], [ <i64 0, i64 0, i64 126, i64 127>, %delay ]
+; CHECK-NOT: select
+; CHECK: ret <4 x i64> %phinode
+ %phinode =  phi <4 x i1> [ <i1 true, i1 true, i1 true, i1 true>, %entry ], [ <i1 true, i1 true, i1 false, i1 false>, %delay ]
+ %sel = select <4 x i1> %phinode, <4 x i64> zeroinitializer, <4 x i64> <i64 124, i64 125, i64 126, i64 127>
+ ret <4 x i64> %sel
+}
+
+define <4 x i64> @vec2(i1 %which) {
+entry:
+  br i1 %which, label %final, label %delay
+
+delay:
+ br label %final
+
+final:
+; CHECK-LABEL: @vec2
+; CHECK-LABEL: final:
+; CHECK: %phinode = phi <4 x i64> [ <i64 124, i64 125, i64 126, i64 127>, %entry ], [ <i64 0, i64 125, i64 0, i64 127>, %delay ]
+; CHECK-NOT: select
+; CHECK: ret <4 x i64> %phinode
+ %phinode =  phi <4 x i1> [ <i1 false, i1 false, i1 false, i1 false>, %entry ], [ <i1 true, i1 false, i1 true, i1 false>, %delay ]
+ %sel = select <4 x i1> %phinode, <4 x i64> zeroinitializer, <4 x i64> <i64 124, i64 125, i64 126, i64 127>
+ ret <4 x i64> %sel
+}
Index: lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- lib/Transforms/InstCombine/InstructionCombining.cpp
+++ lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -908,7 +908,10 @@
       // Beware of ConstantExpr:  it may eventually evaluate to getNullValue,
       // even if currently isNullValue gives false.
       Constant *InC = dyn_cast<Constant>(PN->getIncomingValue(i));
-      if (InC && !isa<ConstantExpr>(InC))
+      // For vector constants, we cannot use isNullValue to fold into
+      // FalseVInPred versus TrueVInPred. It does not investigate the `nullness`
+      // of individual elements in the vector (and will always return false).
+      if (InC && !isa<ConstantExpr>(InC) && !isa<VectorType>(InC->getType()))
         InV = InC->isNullValue() ? FalseVInPred : TrueVInPred;
       else
         InV = Builder->CreateSelect(PN->getIncomingValue(i),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31189.92491.patch
Type: text/x-patch
Size: 2798 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170321/18e60396/attachment.bin>