Skip to content

Commit c590a98

Browse files
committed
[InstCombine] fix potential miscompile in select value equivalence
As shown in the example based on: https://llvm.org/PR49832 ...and the existing test, we can't substitute a vector value because the equality compare replacement that we are attempting requires that the comparison is true for the entire value. Vector select can be partly true/false.
1 parent c0b0da4 commit c590a98

File tree

2 files changed

+11
-5
lines changed

2 files changed

+11
-5
lines changed

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -1124,7 +1124,10 @@ static Instruction *canonicalizeAbsNabs(SelectInst &Sel, ICmpInst &Cmp,
11241124
/// TODO: Wrapping flags could be preserved in some cases with better analysis.
11251125
Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
11261126
ICmpInst &Cmp) {
1127-
if (!Cmp.isEquality())
1127+
// Value equivalence substitution requires an all-or-nothing replacement.
1128+
// It does not make sense for a vector compare where each lane is chosen
1129+
// independently.
1130+
if (!Cmp.isEquality() || Cmp.getType()->isVectorTy())
11281131
return nullptr;
11291132

11301133
// Canonicalize the pattern to ICMP_EQ by swapping the select operands.

llvm/test/Transforms/InstCombine/select-binop-cmp.ll

+7-4
Original file line numberDiff line numberDiff line change
@@ -551,12 +551,12 @@ define i32 @select_xor_icmp_bad_6(i32 %x, i32 %y, i32 %z) {
551551
ret i32 %C
552552
}
553553

554-
; FIXME: Value equivalence substitution is all-or-nothing, so needs a scalar compare.
554+
; Value equivalence substitution is all-or-nothing, so needs a scalar compare.
555555

556556
define <2 x i8> @select_xor_icmp_vec_bad(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z) {
557557
; CHECK-LABEL: @select_xor_icmp_vec_bad(
558558
; CHECK-NEXT: [[A:%.*]] = icmp eq <2 x i8> [[X:%.*]], <i8 5, i8 3>
559-
; CHECK-NEXT: [[B:%.*]] = xor <2 x i8> [[Z:%.*]], <i8 5, i8 3>
559+
; CHECK-NEXT: [[B:%.*]] = xor <2 x i8> [[X]], [[Z:%.*]]
560560
; CHECK-NEXT: [[C:%.*]] = select <2 x i1> [[A]], <2 x i8> [[B]], <2 x i8> [[Y:%.*]]
561561
; CHECK-NEXT: ret <2 x i8> [[C]]
562562
;
@@ -566,11 +566,14 @@ define <2 x i8> @select_xor_icmp_vec_bad(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z)
566566
ret <2 x i8> %C
567567
}
568568

569-
; FIXME: Value equivalence substitution is all-or-nothing, so needs a scalar compare.
569+
; Value equivalence substitution is all-or-nothing, so needs a scalar compare.
570570

571571
define <2 x i32> @vec_select_no_equivalence(<2 x i32> %x) {
572572
; CHECK-LABEL: @vec_select_no_equivalence(
573-
; CHECK-NEXT: ret <2 x i32> [[X:%.*]]
573+
; CHECK-NEXT: [[X10:%.*]] = shufflevector <2 x i32> [[X:%.*]], <2 x i32> undef, <2 x i32> <i32 1, i32 0>
574+
; CHECK-NEXT: [[COND:%.*]] = icmp eq <2 x i32> [[X]], zeroinitializer
575+
; CHECK-NEXT: [[S:%.*]] = select <2 x i1> [[COND]], <2 x i32> [[X10]], <2 x i32> [[X]]
576+
; CHECK-NEXT: ret <2 x i32> [[S]]
574577
;
575578
%x10 = shufflevector <2 x i32> %x, <2 x i32> undef, <2 x i32> <i32 1, i32 0>
576579
%cond = icmp eq <2 x i32> %x, zeroinitializer

0 commit comments

Comments
 (0)