[InstCombine] fix wrong undef handling when converting select to shuffle

[spatel]

Summary

As discussed in:
https://bugs.llvm.org/show_bug.cgi?id=32486
...the canonicalization of vector select to shufflevector does not hold up when undef elements are present in the condition vector.

[efriedma]

docs/LangRef.rst

7077

The second operand may be undef if performing a shuffle from only one vector.

This is technically true, but not really a useful statement. Probably better to explicitly state that an element of the result is undef iff the chosen element of the input is undef.

lib/Transforms/InstCombine/InstCombineSelect.cpp

1059

You could treat the undef as if it were true or false here (just pick arbitrarily), but I'm not sure what the right heuristic would be. You can just leave it for now, I guess.

[spatel]

Patch updated:
Replace a low value statement about shuffle undef-ness in the LangRef.

lib/Transforms/InstCombine/InstCombineSelect.cpp

1059

Right - I thought about that as a follow-up. We can handle the case where all defined elements are coming from one side or the other by choosing the undef elements from the same side. That could be done in instsimplify, so all we're left with here is the case where we are choosing from both sides and have undef elements.

[efriedma]

LGTM.

docs/LangRef.rst

 constant integer or undef values.
 
 Semantics:
 """"""""""
 
 The elements of the two input vectors are numbered from left to right
 across both of the vectors. The shuffle mask operand specifies, for each
 element of the result vector, which element of the two input vectors the
-result element gets. The element selector may be undef (meaning "don't
-care") and the second operand may be undef if performing a shuffle from
-only one vector.
+result element gets. If the shuffle mask is undef, the result vector is
+undef. If any element of the mask operand is undef, that element of the
+result is undef. The second operand may be undef if performing a shuffle
+from only one vector.
 
 Example:
 """"""""
 
 .. code-block:: text
 
       <result> = shufflevector <4 x i32> %v1, <4 x i32> %v2,
                               <4 x i32> <i32 0, i32 4, i32 1, i32 5>  ; yields <4 x i32>

lib/Transforms/InstCombine/InstCombineSelect.cpp

for (unsigned i = 0; i != NumElts; ++i) {
 
     if (Elt->isOneValue()) {
       // If the select condition element is true, choose from the 1st vector.
       Mask.push_back(ConstantInt::get(Int32Ty, i));
     } else if (Elt->isNullValue()) {
       // If the select condition element is false, choose from the 2nd vector.
       Mask.push_back(ConstantInt::get(Int32Ty, i + NumElts));
     } else if (isa<UndefValue>(Elt)) {
-      // If the select condition element is undef, the shuffle mask is undef.
-      Mask.push_back(UndefValue::get(Int32Ty));
+      // Undef in a select condition (choose one of the operands) does not mean
+      // the same thing as undef in a shuffle mask (any value is acceptable), so
+      // give up.
+      return nullptr;
     } else {
       // Bail out on a constant expression.
       return nullptr;
     }
   }
 
   return new ShuffleVectorInst(SI.getTrueValue(), SI.getFalseValue(),
                                ConstantVector::get(Mask));

test/Transforms/InstCombine/select.ll

 ; 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
 }
 
-; Make sure that undef elements of the select condition are translated into undef elements of the shuffle mask.
-
 define <4 x i32> @canonicalize_to_shuffle(<4 x i32> %a, <4 x i32> %b) {
 ; CHECK-LABEL: @canonicalize_to_shuffle(
-; CHECK-NEXT:    [[SEL:%.*]] = shufflevector <4 x i32> %a, <4 x i32> %b, <4 x i32> <i32 0, i32 undef, i32 6, i32 undef>
+; CHECK-NEXT:    [[SEL:%.*]] = shufflevector <4 x i32> %a, <4 x i32> %b, <4 x i32> <i32 0, i32 5, i32 6, i32 3>
+; CHECK-NEXT:    ret <4 x i32> [[SEL]]
+;
+  %sel = select <4 x i1> <i1 true, i1 false, i1 false, i1 true>, <4 x i32> %a, <4 x i32> %b
+  ret <4 x i32> %sel
+}
+
+; Undef elements of the select condition may not be translated into undef elements of a shuffle mask
+; because undef in a shuffle mask means we can return anything, not just one of the selected values.
+; https://bugs.llvm.org/show_bug.cgi?id=32486
+
+define <4 x i32> @undef_elts_in_condition(<4 x i32> %a, <4 x i32> %b) {
+; CHECK-LABEL: @undef_elts_in_condition(
+; CHECK-NEXT:    [[SEL:%.*]] = select <4 x i1> <i1 true, i1 undef, i1 false, i1 undef>, <4 x i32> %a, <4 x i32> %b
 ; CHECK-NEXT:    ret <4 x i32> [[SEL]]
 ;
   %sel = select <4 x i1> <i1 true, i1 undef, i1 false, i1 undef>, <4 x i32> %a, <4 x i32> %b
   ret <4 x i32> %sel
 }
 
 ; Don't die or try if the condition mask is a constant expression or contains a constant expression.