[InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

[craig.topper]

Summary

As noted here https://lists.llvm.org/pipermail/llvm-dev/2016-October/106182.html and by alive2, this transform isn't valid. If X is poison this potentially propagates poison when it shouldn't.

It seems we don't have any tests for this in either InstSimplify or InstCombine. I can add negative tests if we want.

The same transform also exists in DAGCombiner.

[lebedev.ri]

I do think we should have (miscompiled) tests in all places first.

[nlopes]

Here's an end-to-end miscompilation: https://bugs.llvm.org/show_bug.cgi?id=31633

[lebedev.ri]

In D83360#2138386, @nlopes wrote:

Here's an end-to-end miscompilation: https://bugs.llvm.org/show_bug.cgi?id=31633

Sure, but we still need to have a test with comment that it should not be folded, referencing all this.

[spatel]

In D83360#2138392, @lebedev.ri wrote:

In D83360#2138386, @nlopes wrote:

Here's an end-to-end miscompilation: https://bugs.llvm.org/show_bug.cgi?id=31633

Sure, but we still need to have a test with comment that it should not be folded, referencing all this.

Yes - we need ~4 tests ( {trueval/falseval} * {scalar/vector} ) to ensure that the transform doesn't get mistakenly re-added. Also, there's a clang test that is going to fail with this change:
clang/test/CodeGen/arm-mve-intrinsics/dup.c
That file tried to be minimally dependent on opt, but still used -early-cse which calls instsimplify for analysis.

[craig.topper]

Add tests for InstCombine and InstSimplfy.
Update frontend test

[spatel]

LGTM

[lebedev.ri]

LG, thank you.

[efriedma]

Please also add testcases with select constant expressions, to test constant folding.

[craig.topper]

In D83360#2139933, @efriedma wrote:

Please also add testcases with select constant expressions, to test constant folding.

Should we remove the handling from llvm::ConstantFoldSelectInstruction

[efriedma]

Should we remove the handling from llvm::ConstantFoldSelectInstruction

It seems silly to remove the handling from InstSimplify, but not constant folding.

[majnemer]

llvm/lib/Analysis/InstructionSimplify.cpp

4120–4124

Can we still do these optimizations when X is a frozen value?

[regehr]

@majnemer should work: https://alive2.llvm.org/ce/z/vL4yn4

[craig.topper]

Wasn't @majnemer asking about

define i32 @src(i1 %cond, i32 %x) {
  %xf = freeze i32 %x
  %s = select i1 %cond, i32 %xf, i32 undef
  ret i32 %s
}

which is legal. I'm going to work on supporting known non-poison cases.

[regehr]

@craig.topper ok, I agree that should work. alive doesn't like it -- is this an alive bug @nlopes? a freeze should not yield undef.
https://alive2.llvm.org/ce/z/mWAsYv

[efriedma]

In D83360#2140224, @regehr wrote:

@craig.topper ok, I agree that should work. alive doesn't like it -- is this an alive bug @nlopes? a freeze should not yield undef.
https://alive2.llvm.org/ce/z/mWAsYv

Did you mean to check something like the following?

define i32 @src(i1 %cond, i32 %x) {
  %x2 = freeze i32 %x
  %s = select i1 %cond, i32 %x2, i32 undef
  ret i32 %s
}

define i32 @tgt(i1 %cond, i32 %x) {
  %x2 = freeze i32 %x
  ret i32 %x2
}

[craig.topper]

Alive does like this https://alive2.llvm.org/ce/z/yhibbe which is what I was going to implement.

[nlopes]

In D83360#2140241, @craig.topper wrote:

Alive does like this https://alive2.llvm.org/ce/z/yhibbe which is what I was going to implement.

right. There's a guaranteedNonPoison (or similar name) in ValueTracking that can be used I guess.

[regehr]

Did you mean to check something like the following?

define i32 @src(i1 %cond, i32 %x) {
  %x2 = freeze i32 %x
  %s = select i1 %cond, i32 %x2, i32 undef
  ret i32 %s
}

define i32 @tgt(i1 %cond, i32 %x) {
  %x2 = freeze i32 %x
  ret i32 %x2
}

that's fine but I still don't understand why the counterexample to my version says %x2 in @src can be undef

[efriedma]

that's fine but I still don't understand why the counterexample to my version says %x2 in @src can be undef

If I'm understanding correctly, this reduces to something like the following:

define i32 @src() {

%x2 = freeze i32 undef
ret i32 %x2

}

define i32 @tgt() {

ret i32 undef

}

This seems a little suspect, yes.

[nlopes]

In D83360#2142457, @efriedma wrote:

that's fine but I still don't understand why the counterexample to my version says %x2 in @src can be undef

If I'm understanding correctly, this reduces to something like the following:

define i32 @src() {

%x2 = freeze i32 undef
ret i32 %x2

}

define i32 @tgt() {

ret i32 undef

}

This seems a little suspect, yes.

This is a known bug: https://github.com/AliveToolkit/alive2/issues/3
gotta fix this soon.

[vitalybuka]

After this patch we have false msan reports on code like this:

bool iv_compare2(const int *op1, const int *op2) {
  if (op1[1] != op2[1]) return op1[1] < op2[1];
  for (int i = 1; i >= 0; i--) {
    if (op1[i] != op2[i]) return op1[i] < op2[i];
  }
  return false;
}

void foo() {
  int a[2] = {};
  int b[2] = {};
  auto UNINITIALIZED= iv_compare2(a, b);
}

Here it looks fine and the same as before the patch. It returns "and undef, false" which should be false.

*** IR Dump After Simplify the CFG ***
; Function Attrs: norecurse nounwind readonly sanitize_memory uwtable
define zeroext i1 @_Z11iv_compare2PKiS0_(i32* nocapture readonly %op1, i32* nocapture readonly %op2) local_unnamed_addr #0 !dbg !8 {
entry:
  %arrayidx = getelementptr inbounds i32, i32* %op1, i64 1, !dbg !10
  %0 = load i32, i32* %arrayidx, align 4, !dbg !10
  %arrayidx1 = getelementptr inbounds i32, i32* %op2, i64 1, !dbg !11
  %1 = load i32, i32* %arrayidx1, align 4, !dbg !11
  %cmp.not = icmp eq i32 %0, %1, !dbg !12
  br i1 %cmp.not, label %for.cond, label %if.then, !dbg !10

if.then:                                          ; preds = %entry
  %cmp4 = icmp slt i32 %0, %1, !dbg !13
  ret i1 %cmp4, !dbg !14

for.cond:                                         ; preds = %entry
  %2 = load i32, i32* %op1, align 4, !dbg !15
  %3 = load i32, i32* %op2, align 4, !dbg !16
  %cmp9.not.1 = icmp eq i32 %2, %3, !dbg !17
  %cmp15 = icmp slt i32 %2, %3
  %spec.select39 = select i1 %cmp9.not.1, i1 undef, i1 %cmp15, !dbg !15
  %spec.select40 = select i1 %cmp9.not.1, i1 false, i1 true, !dbg !15
  %spec.select = and i1 %spec.select39, %spec.select40
  ret i1 %spec.select
}

However with this patch after the next transformation it breaks the code:
Now it returns undef instead of false if %2 == %3

*** IR Dump After Combine redundant instructions ***
; Function Attrs: norecurse nounwind readonly sanitize_memory uwtable
define zeroext i1 @_Z11iv_compare2PKiS0_(i32* nocapture readonly %op1, i32* nocapture readonly %op2) local_unnamed_addr #0 !dbg !8 {
entry:
  %arrayidx = getelementptr inbounds i32, i32* %op1, i64 1, !dbg !10
  %0 = load i32, i32* %arrayidx, align 4, !dbg !10
  %arrayidx1 = getelementptr inbounds i32, i32* %op2, i64 1, !dbg !11
  %1 = load i32, i32* %arrayidx1, align 4, !dbg !11
  %cmp.not = icmp eq i32 %0, %1, !dbg !12
  br i1 %cmp.not, label %for.cond, label %if.then, !dbg !10

if.then:                                          ; preds = %entry
  %cmp4 = icmp slt i32 %0, %1, !dbg !13
  ret i1 %cmp4, !dbg !14

for.cond:                                         ; preds = %entry
  %2 = load i32, i32* %op1, align 4, !dbg !15
  %3 = load i32, i32* %op2, align 4, !dbg !16
  %cmp9.not.1 = icmp eq i32 %2, %3, !dbg !17
  %cmp15 = icmp slt i32 %2, %3
  %spec.select39 = select i1 %cmp9.not.1, i1 undef, i1 %cmp15, !dbg !15
  ret i1 %spec.select39
}

Then msan reasonably reports a bug.

[aqjune]

Seems like a bug in instsimplify:

define i1 @f(i32 %x, i32 %y) {
  %cmp9.not.1 = icmp eq i32 %x, %y
  %cmp15      = icmp slt i32 %x, %y
  %spec.select39 = select i1 %cmp9.not.1, i1 undef, i1 %cmp15
  %spec.select40 = xor i1 %cmp9.not.1, 1
  %spec.select  = and i1 %spec.select39, %spec.select40
  ret i1 %spec.select
}
=>
define i1 @f(i32 %x, i32 %y) {
  %cmp9.not.1 = icmp eq i32 %x, %y
  %cmp15 = icmp slt i32 %x, %y
  %spec.select39 = select i1 %cmp9.not.1, i1 undef, i1 %cmp15
  ret i1 %spec.select39
}

https://godbolt.org/z/a8f7hT

Alive2 says it's incorrect: https://alive2.llvm.org/ce/z/-8Q4HL

Seems to be related with ValueTracking's isImpliedCondition since this optimizations happens only when operands of the two icmps are the same.

[aqjune]

(renaming variables for readability)

%a = select i1 %s, i1 undef, i1 %t
%b = xor i1 %s, 1
%c = and i1 %a, %b

This series of reasoning happened from a single SimplifyAndInst call:

c = a & (s ^ 1)
  = (a & s) ^ (a & 1)                    ; ExpandBinOp
  = ((select s, undef, t) & s) ^ a
  = (select s, (undef & s), (t & s)) ^ a ; ThreadBinOpOverSelect
  = (select s, (undef & s), false) ^ a   ; since s = (x == y), t = (x < y)
  = (select s, false, false) ^ a         ; choose undef to be false
  = a
  = select s, undef, t

In general, distributing a into operands of xor (second line) isn't sound because it increases the number of uses of a. We don't want to totally disable the simplification, however.

If InstSimplify never increases the number of uses in the end, we have an alternative solution: tracking to which value undef is folded.
Whenever an undef value is chosen to be a concrete value, the decision should be remembered, so the copied undefs won't be folded into different values.
In case of InstSimplify, we can identify individual undefs by Use, since InstSimplify won't do any transformation inside.
This means SimplifyXXX needs to return two things: the simplified value & the undef cache. Since InstSimplify isn't designed to do transformation directly, other optimizations like InstCombine should perform the final change.

Does this solution make sense? Then, I can prepare a patch for this.

[nlopes]

FYI InstSimplify doing distribution of undef is a known bug: https://bugs.llvm.org/show_bug.cgi?id=33165

[echristo]

This seems like something that we should then revert until we know that instsimplify can be updated with a fix?

[lebedev.ri]

In D83360#2154540, @echristo wrote:

This seems like something that we should then revert until we know that instsimplify can be updated with a fix?

Possibility for a miscompile sounds much worse to me than a false-positive diag..

[echristo]

We're starting to see miscompiles as we do more testing as well, just nothing smaller at the moment.

[lebedev.ri]

In D83360#2154545, @echristo wrote:

We're starting to see miscompiles as we do more testing as well, just nothing smaller at the moment.

Could you clarify, do you mean that this fix is causing (new) miscompiles?

[echristo]

It's that even before the msan instrumentation the IR doesn't look correct - thus a miscompile.

[aqjune]

In D83360#2154584, @echristo wrote:

It's that even before the msan instrumentation the IR doesn't look correct - thus a miscompile.

I'll share a prototype of the InstSimplify patch on Phabricator, in a day or two; it would be great if the miscompilation is fixed with the patch.

[echristo]

In D83360#2154637, @aqjune wrote:

In D83360#2154584, @echristo wrote:

It's that even before the msan instrumentation the IR doesn't look correct - thus a miscompile.

I'll share a prototype of the InstSimplify patch on Phabricator, in a day or two; it would be great if the miscompilation is fixed with the patch.

Would it be reasonable to revert this in the meantime?

[craig.topper]

I'm going to revert this as Eric requested. And I'll ask to merge the revert to the 11 branch.

[craig.topper]

Reverted in 00f3579aea6e3d4a4b7464c3db47294f71cef9e4

[craig.topper]

Merge request for 11.0 https://bugs.llvm.org/show_bug.cgi?id=46740

[craig.topper]

@aqjune did you put a patch for InstSimplify doing distribution over undef yet?

[aqjune]

In D83360#2162898, @craig.topper wrote:

@aqjune did you put a patch for InstSimplify doing distribution over undef yet?

Sorry, making InstSimplify to safely distribute undef was a nontrivial job - other than updating InstSimplify to track uses, it needed rewinding folded undef records if simplification failed & update constant folding to track uses too. The folded value could be symbolic, making things more complex.
I'm trying an alternative solution for this problem, I will leave a link here after submission.

[aqjune]

Made a patch at https://reviews.llvm.org/D84250 .

[nikic]

D85684 has landed, so we can try reapplying this change.

[craig.topper]

In D83360#2210630, @nikic wrote:

D85684 has landed, so we can try reapplying this change.

I've put up D85765 with this patch and the follow ups re-enabled.

[craig.topper]

This got replaced by D85765

clang/test/CodeGen/arm-mve-intrinsics/dup.c

 }
 
 // CHECK-LABEL: @test_vdupq_x_n_f16(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = zext i16 [[P:%.*]] to i32
 // CHECK-NEXT:    [[TMP1:%.*]] = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 [[TMP0]])
 // CHECK-NEXT:    [[DOTSPLATINSERT:%.*]] = insertelement <8 x half> undef, half [[A:%.*]], i32 0
 // CHECK-NEXT:    [[DOTSPLAT:%.*]] = shufflevector <8 x half> [[DOTSPLATINSERT]], <8 x half> undef, <8 x i32> zeroinitializer
-// CHECK-NEXT:    ret <8 x half> [[DOTSPLAT]]
+// CHECK-NEXT:    [[TMP2:%.*]] = select <8 x i1> [[TMP1]], <8 x half> [[DOTSPLAT]], <8 x half> undef
+// CHECK-NEXT:    ret <8 x half> [[TMP2]]
 //
 float16x8_t test_vdupq_x_n_f16(float16_t a, mve_pred16_t p)
 {
     return vdupq_x_n_f16(a, p);
 }
 
 // CHECK-LABEL: @test_vdupq_x_n_f32(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = zext i16 [[P:%.*]] to i32
 // CHECK-NEXT:    [[TMP1:%.*]] = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 [[TMP0]])
 // CHECK-NEXT:    [[DOTSPLATINSERT:%.*]] = insertelement <4 x float> undef, float [[A:%.*]], i32 0
 // CHECK-NEXT:    [[DOTSPLAT:%.*]] = shufflevector <4 x float> [[DOTSPLATINSERT]], <4 x float> undef, <4 x i32> zeroinitializer
-// CHECK-NEXT:    ret <4 x float> [[DOTSPLAT]]
+// CHECK-NEXT:    [[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x float> [[DOTSPLAT]], <4 x float> undef
+// CHECK-NEXT:    ret <4 x float> [[TMP2]]
 //
 float32x4_t test_vdupq_x_n_f32(float32_t a, mve_pred16_t p)
 {
     return vdupq_x_n_f32(a, p);
 }
 
 // CHECK-LABEL: @test_vdupq_x_n_s8(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = zext i16 [[P:%.*]] to i32
 // CHECK-NEXT:    [[TMP1:%.*]] = call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 [[TMP0]])
 // CHECK-NEXT:    [[DOTSPLATINSERT:%.*]] = insertelement <16 x i8> undef, i8 [[A:%.*]], i32 0
 // CHECK-NEXT:    [[DOTSPLAT:%.*]] = shufflevector <16 x i8> [[DOTSPLATINSERT]], <16 x i8> undef, <16 x i32> zeroinitializer
-// CHECK-NEXT:    ret <16 x i8> [[DOTSPLAT]]
+// CHECK-NEXT:    [[TMP2:%.*]] = select <16 x i1> [[TMP1]], <16 x i8> [[DOTSPLAT]], <16 x i8> undef
+// CHECK-NEXT:    ret <16 x i8> [[TMP2]]
 //
 int8x16_t test_vdupq_x_n_s8(int8_t a, mve_pred16_t p)
 {
     return vdupq_x_n_s8(a, p);
 }
 
 // CHECK-LABEL: @test_vdupq_x_n_s16(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = zext i16 [[P:%.*]] to i32
 // CHECK-NEXT:    [[TMP1:%.*]] = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 [[TMP0]])
 // CHECK-NEXT:    [[DOTSPLATINSERT:%.*]] = insertelement <8 x i16> undef, i16 [[A:%.*]], i32 0
 // CHECK-NEXT:    [[DOTSPLAT:%.*]] = shufflevector <8 x i16> [[DOTSPLATINSERT]], <8 x i16> undef, <8 x i32> zeroinitializer
-// CHECK-NEXT:    ret <8 x i16> [[DOTSPLAT]]
+// CHECK-NEXT:    [[TMP2:%.*]] = select <8 x i1> [[TMP1]], <8 x i16> [[DOTSPLAT]], <8 x i16> undef
+// CHECK-NEXT:    ret <8 x i16> [[TMP2]]
 //
 int16x8_t test_vdupq_x_n_s16(int16_t a, mve_pred16_t p)
 {
     return vdupq_x_n_s16(a, p);
 }
 
 // CHECK-LABEL: @test_vdupq_x_n_s32(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = zext i16 [[P:%.*]] to i32
 // CHECK-NEXT:    [[TMP1:%.*]] = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 [[TMP0]])
 // CHECK-NEXT:    [[DOTSPLATINSERT:%.*]] = insertelement <4 x i32> undef, i32 [[A:%.*]], i32 0
 // CHECK-NEXT:    [[DOTSPLAT:%.*]] = shufflevector <4 x i32> [[DOTSPLATINSERT]], <4 x i32> undef, <4 x i32> zeroinitializer
-// CHECK-NEXT:    ret <4 x i32> [[DOTSPLAT]]
+// CHECK-NEXT:    [[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x i32> [[DOTSPLAT]], <4 x i32> undef
+// CHECK-NEXT:    ret <4 x i32> [[TMP2]]
 //
 int32x4_t test_vdupq_x_n_s32(int32_t a, mve_pred16_t p)
 {
     return vdupq_x_n_s32(a, p);
 }
 
 // CHECK-LABEL: @test_vdupq_x_n_u8(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = zext i16 [[P:%.*]] to i32
 // CHECK-NEXT:    [[TMP1:%.*]] = call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 [[TMP0]])
 // CHECK-NEXT:    [[DOTSPLATINSERT:%.*]] = insertelement <16 x i8> undef, i8 [[A:%.*]], i32 0
 // CHECK-NEXT:    [[DOTSPLAT:%.*]] = shufflevector <16 x i8> [[DOTSPLATINSERT]], <16 x i8> undef, <16 x i32> zeroinitializer
-// CHECK-NEXT:    ret <16 x i8> [[DOTSPLAT]]
+// CHECK-NEXT:    [[TMP2:%.*]] = select <16 x i1> [[TMP1]], <16 x i8> [[DOTSPLAT]], <16 x i8> undef
+// CHECK-NEXT:    ret <16 x i8> [[TMP2]]
 //
 uint8x16_t test_vdupq_x_n_u8(uint8_t a, mve_pred16_t p)
 {
     return vdupq_x_n_u8(a, p);
 }
 
 // CHECK-LABEL: @test_vdupq_x_n_u16(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = zext i16 [[P:%.*]] to i32
 // CHECK-NEXT:    [[TMP1:%.*]] = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 [[TMP0]])
 // CHECK-NEXT:    [[DOTSPLATINSERT:%.*]] = insertelement <8 x i16> undef, i16 [[A:%.*]], i32 0
 // CHECK-NEXT:    [[DOTSPLAT:%.*]] = shufflevector <8 x i16> [[DOTSPLATINSERT]], <8 x i16> undef, <8 x i32> zeroinitializer
-// CHECK-NEXT:    ret <8 x i16> [[DOTSPLAT]]
+// CHECK-NEXT:    [[TMP2:%.*]] = select <8 x i1> [[TMP1]], <8 x i16> [[DOTSPLAT]], <8 x i16> undef
+// CHECK-NEXT:    ret <8 x i16> [[TMP2]]
 //
 uint16x8_t test_vdupq_x_n_u16(uint16_t a, mve_pred16_t p)
 {
     return vdupq_x_n_u16(a, p);
 }
 
 // CHECK-LABEL: @test_vdupq_x_n_u32(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = zext i16 [[P:%.*]] to i32
 // CHECK-NEXT:    [[TMP1:%.*]] = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 [[TMP0]])
 // CHECK-NEXT:    [[DOTSPLATINSERT:%.*]] = insertelement <4 x i32> undef, i32 [[A:%.*]], i32 0
 // CHECK-NEXT:    [[DOTSPLAT:%.*]] = shufflevector <4 x i32> [[DOTSPLATINSERT]], <4 x i32> undef, <4 x i32> zeroinitializer
-// CHECK-NEXT:    ret <4 x i32> [[DOTSPLAT]]
+// CHECK-NEXT:    [[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x i32> [[DOTSPLAT]], <4 x i32> undef
+// CHECK-NEXT:    ret <4 x i32> [[TMP2]]
 //
 uint32x4_t test_vdupq_x_n_u32(uint32_t a, mve_pred16_t p)
 {
     return vdupq_x_n_u32(a, p);
 }
 

llvm/lib/Analysis/InstructionSimplify.cpp

static Value *SimplifySelectInst(Value *Cond, Value *TrueVal, Value *FalseVal,
   if (Cond->getType() == TrueVal->getType() &&
       match(TrueVal, m_One()) && match(FalseVal, m_ZeroInt()))
     return Cond;
 
   // select ?, X, X -> X
   if (TrueVal == FalseVal)
     return TrueVal;
 
-  if (isa<UndefValue>(TrueVal))   // select ?, undef, X -> X
-    return FalseVal;
-  if (isa<UndefValue>(FalseVal))   // select ?, X, undef -> X
-    return TrueVal;
-
   // Deal with partial undef vector constants: select ?, VecC, VecC' --> VecC''
   Constant *TrueC, *FalseC;
   if (TrueVal->getType()->isVectorTy() && match(TrueVal, m_Constant(TrueC)) &&
       match(FalseVal, m_Constant(FalseC))) {
     unsigned NumElts = cast<VectorType>(TrueC->getType())->getNumElements();
     SmallVector<Constant *, 16> NewC;
     for (unsigned i = 0; i != NumElts; ++i) {
       // Bail out on incomplete vector constants.

llvm/test/Transforms/InstCombine/select.ll

 merge:
   %phi = phi i32 [%A, %if.true], [%B, %if.false]
   br label %exit
 
 exit:
   %sel = select i1 %cond, i32 %phi, i32 %A
   ret i32 %sel
 }
+
+; Negative tests to ensure we don't remove selects with undef true/false values.
+; See https://bugs.llvm.org/show_bug.cgi?id=31633
+; https://lists.llvm.org/pipermail/llvm-dev/2016-October/106182.html
+; https://reviews.llvm.org/D83360
+define i32 @false_undef(i1 %cond, i32 %x) {
+; CHECK-LABEL: @false_undef(
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[COND:%.*]], i32 [[X:%.*]], i32 undef
+; CHECK-NEXT:    ret i32 [[S]]
+;
+  %s = select i1 %cond, i32 %x, i32 undef
+  ret i32 %s
+}
+
+define i32 @true_undef(i1 %cond, i32 %x) {
+; CHECK-LABEL: @true_undef(
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[COND:%.*]], i32 undef, i32 [[X:%.*]]
+; CHECK-NEXT:    ret i32 [[S]]
+;
+  %s = select i1 %cond, i32 undef, i32 %x
+  ret i32 %s
+}
+
+define <2 x i32> @false_undef_vec(i1 %cond, <2 x i32> %x) {
+; CHECK-LABEL: @false_undef_vec(
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[COND:%.*]], <2 x i32> [[X:%.*]], <2 x i32> undef
+; CHECK-NEXT:    ret <2 x i32> [[S]]
+;
+  %s = select i1 %cond, <2 x i32> %x, <2 x i32> undef
+  ret <2 x i32> %s
+}
+
+define <2 x i32> @true_undef_vec(i1 %cond, <2 x i32> %x) {
+; CHECK-LABEL: @true_undef_vec(
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[COND:%.*]], <2 x i32> undef, <2 x i32> [[X:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[S]]
+;
+  %s = select i1 %cond, <2 x i32> undef, <2 x i32> %x
+  ret <2 x i32> %s
+}

llvm/test/Transforms/InstSimplify/select.ll

 ; CHECK-NEXT:    [[C3:%.*]] = select i1 [[C1]], i1 [[C2]], i1 false
 ; CHECK-NEXT:    ret i1 [[C3]]
 ;
   %c1 = fcmp ord float 0.0, %x
   %c2 = fcmp ord float %x, %y
   %c3 = select i1 %c1, i1 %c2, i1 false
   ret i1 %c3
 }
+
+; Negative tests to ensure we don't remove selects with undef true/false values.
+; See https://bugs.llvm.org/show_bug.cgi?id=31633
+; https://lists.llvm.org/pipermail/llvm-dev/2016-October/106182.html
+; https://reviews.llvm.org/D83360
+define i32 @false_undef(i1 %cond, i32 %x) {
+; CHECK-LABEL: @false_undef(
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[COND:%.*]], i32 [[X:%.*]], i32 undef
+; CHECK-NEXT:    ret i32 [[S]]
+;
+  %s = select i1 %cond, i32 %x, i32 undef
+  ret i32 %s
+}
+
+define i32 @true_undef(i1 %cond, i32 %x) {
+; CHECK-LABEL: @true_undef(
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[COND:%.*]], i32 undef, i32 [[X:%.*]]
+; CHECK-NEXT:    ret i32 [[S]]
+;
+  %s = select i1 %cond, i32 undef, i32 %x
+  ret i32 %s
+}
+
+define <2 x i32> @false_undef_vec(i1 %cond, <2 x i32> %x) {
+; CHECK-LABEL: @false_undef_vec(
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[COND:%.*]], <2 x i32> [[X:%.*]], <2 x i32> undef
+; CHECK-NEXT:    ret <2 x i32> [[S]]
+;
+  %s = select i1 %cond, <2 x i32> %x, <2 x i32> undef
+  ret <2 x i32> %s
+}
+
+define <2 x i32> @true_undef_vec(i1 %cond, <2 x i32> %x) {
+; CHECK-LABEL: @true_undef_vec(
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[COND:%.*]], <2 x i32> undef, <2 x i32> [[X:%.*]]
+; CHECK-NEXT:    ret <2 x i32> [[S]]
+;
+  %s = select i1 %cond, <2 x i32> undef, <2 x i32> %x
+  ret <2 x i32> %s
+}