Skip to content

Commit 7339f7b

Browse files
authored
[InstCombine] Fix poison propagation in select of bitwise fold (#89701)
We're replacing the select with the false value here, but it may be more poisonous if m_Not contains poison elements. Fix this by introducing a m_NotForbidPoison matcher and using it here. Fixes #89500.
1 parent a1b1c4a commit 7339f7b

File tree

4 files changed

+41
-15
lines changed

4 files changed

+41
-15
lines changed

llvm/include/llvm/IR/PatternMatch.h

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,9 @@ template <int64_t Val> inline constantint_match<Val> m_ConstantInt() {
350350

351351
/// This helper class is used to match constant scalars, vector splats,
352352
/// and fixed width vectors that satisfy a specified predicate.
353-
/// For fixed width vector constants, poison elements are ignored.
354-
template <typename Predicate, typename ConstantVal>
353+
/// For fixed width vector constants, poison elements are ignored if AllowPoison
354+
/// is true.
355+
template <typename Predicate, typename ConstantVal, bool AllowPoison>
355356
struct cstval_pred_ty : public Predicate {
356357
template <typename ITy> bool match(ITy *V) {
357358
if (const auto *CV = dyn_cast<ConstantVal>(V))
@@ -374,7 +375,7 @@ struct cstval_pred_ty : public Predicate {
374375
Constant *Elt = C->getAggregateElement(i);
375376
if (!Elt)
376377
return false;
377-
if (isa<PoisonValue>(Elt))
378+
if (AllowPoison && isa<PoisonValue>(Elt))
378379
continue;
379380
auto *CV = dyn_cast<ConstantVal>(Elt);
380381
if (!CV || !this->isValue(CV->getValue()))
@@ -389,12 +390,13 @@ struct cstval_pred_ty : public Predicate {
389390
};
390391

391392
/// specialization of cstval_pred_ty for ConstantInt
392-
template <typename Predicate>
393-
using cst_pred_ty = cstval_pred_ty<Predicate, ConstantInt>;
393+
template <typename Predicate, bool AllowPoison = true>
394+
using cst_pred_ty = cstval_pred_ty<Predicate, ConstantInt, AllowPoison>;
394395

395396
/// specialization of cstval_pred_ty for ConstantFP
396397
template <typename Predicate>
397-
using cstfp_pred_ty = cstval_pred_ty<Predicate, ConstantFP>;
398+
using cstfp_pred_ty = cstval_pred_ty<Predicate, ConstantFP,
399+
/*AllowPoison=*/true>;
398400

399401
/// This helper class is used to match scalar and vector constants that
400402
/// satisfy a specified predicate, and bind them to an APInt.
@@ -484,6 +486,10 @@ inline cst_pred_ty<is_all_ones> m_AllOnes() {
484486
return cst_pred_ty<is_all_ones>();
485487
}
486488

489+
inline cst_pred_ty<is_all_ones, false> m_AllOnesForbidPoison() {
490+
return cst_pred_ty<is_all_ones, false>();
491+
}
492+
487493
struct is_maxsignedvalue {
488494
bool isValue(const APInt &C) { return C.isMaxSignedValue(); }
489495
};
@@ -2596,6 +2602,13 @@ m_Not(const ValTy &V) {
25962602
return m_c_Xor(m_AllOnes(), V);
25972603
}
25982604

2605+
template <typename ValTy>
2606+
inline BinaryOp_match<cst_pred_ty<is_all_ones, false>, ValTy, Instruction::Xor,
2607+
true>
2608+
m_NotForbidPoison(const ValTy &V) {
2609+
return m_c_Xor(m_AllOnesForbidPoison(), V);
2610+
}
2611+
25992612
/// Matches an SMin with LHS and RHS in either order.
26002613
template <typename LHS, typename RHS>
26012614
inline MaxMin_match<ICmpInst, LHS, RHS, smin_pred_ty, true>

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,11 +1722,11 @@ static Instruction *foldSelectICmpEq(SelectInst &SI, ICmpInst *ICI,
17221722
return match(CmpRHS, m_Zero()) && match(FalseVal, matchInner);
17231723

17241724
if (NotMask == NotInner) {
1725-
return match(FalseVal,
1726-
m_c_BinOp(OuterOpc, m_Not(matchInner), m_Specific(CmpRHS)));
1725+
return match(FalseVal, m_c_BinOp(OuterOpc, m_NotForbidPoison(matchInner),
1726+
m_Specific(CmpRHS)));
17271727
} else if (NotMask == NotRHS) {
1728-
return match(FalseVal,
1729-
m_c_BinOp(OuterOpc, matchInner, m_Not(m_Specific(CmpRHS))));
1728+
return match(FalseVal, m_c_BinOp(OuterOpc, matchInner,
1729+
m_NotForbidPoison(m_Specific(CmpRHS))));
17301730
} else {
17311731
return match(FalseVal,
17321732
m_c_BinOp(OuterOpc, matchInner, m_Specific(CmpRHS)));

llvm/test/Transforms/InstCombine/select.ll

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3830,14 +3830,17 @@ entry:
38303830
ret i32 %cond
38313831
}
38323832

3833-
; FIXME: This is a miscompile.
38343833
define <2 x i32> @src_and_eq_C_xor_OrAndNotC_vec_poison(<2 x i32> %0, <2 x i32> %1, <2 x i32> %2) {
38353834
; CHECK-LABEL: @src_and_eq_C_xor_OrAndNotC_vec_poison(
38363835
; CHECK-NEXT: entry:
3837-
; CHECK-NEXT: [[OR:%.*]] = or <2 x i32> [[TMP1:%.*]], [[TMP0:%.*]]
3838-
; CHECK-NEXT: [[NOT:%.*]] = xor <2 x i32> [[TMP2:%.*]], <i32 -1, i32 poison>
3836+
; CHECK-NEXT: [[AND:%.*]] = and <2 x i32> [[TMP1:%.*]], [[TMP0:%.*]]
3837+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq <2 x i32> [[AND]], [[TMP2:%.*]]
3838+
; CHECK-NEXT: [[XOR:%.*]] = xor <2 x i32> [[TMP1]], [[TMP0]]
3839+
; CHECK-NEXT: [[OR:%.*]] = or <2 x i32> [[TMP1]], [[TMP0]]
3840+
; CHECK-NEXT: [[NOT:%.*]] = xor <2 x i32> [[TMP2]], <i32 -1, i32 poison>
38393841
; CHECK-NEXT: [[AND1:%.*]] = and <2 x i32> [[OR]], [[NOT]]
3840-
; CHECK-NEXT: ret <2 x i32> [[AND1]]
3842+
; CHECK-NEXT: [[COND:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[XOR]], <2 x i32> [[AND1]]
3843+
; CHECK-NEXT: ret <2 x i32> [[COND]]
38413844
;
38423845
entry:
38433846
%and = and <2 x i32> %1, %0

llvm/unittests/IR/PatternMatch.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1995,7 +1995,7 @@ TEST_F(PatternMatchTest, VScale) {
19951995
EXPECT_TRUE(match(PtrToInt2, m_VScale()));
19961996
}
19971997

1998-
TEST_F(PatternMatchTest, NotForbidUndef) {
1998+
TEST_F(PatternMatchTest, NotForbidPoison) {
19991999
Type *ScalarTy = IRB.getInt8Ty();
20002000
Type *VectorTy = FixedVectorType::get(ScalarTy, 3);
20012001
Constant *ScalarUndef = UndefValue::get(ScalarTy);
@@ -2020,23 +2020,33 @@ TEST_F(PatternMatchTest, NotForbidUndef) {
20202020
Value *X;
20212021
EXPECT_TRUE(match(Not, m_Not(m_Value(X))));
20222022
EXPECT_TRUE(match(X, m_Zero()));
2023+
X = nullptr;
2024+
EXPECT_TRUE(match(Not, m_NotForbidPoison(m_Value(X))));
2025+
EXPECT_TRUE(match(X, m_Zero()));
20232026

20242027
Value *NotCommute = IRB.CreateXor(VectorOnes, VectorZero);
20252028
Value *Y;
20262029
EXPECT_TRUE(match(NotCommute, m_Not(m_Value(Y))));
20272030
EXPECT_TRUE(match(Y, m_Zero()));
2031+
Y = nullptr;
2032+
EXPECT_TRUE(match(NotCommute, m_NotForbidPoison(m_Value(Y))));
2033+
EXPECT_TRUE(match(Y, m_Zero()));
20282034

20292035
Value *NotWithUndefs = IRB.CreateXor(VectorZero, VectorMixedUndef);
20302036
EXPECT_FALSE(match(NotWithUndefs, m_Not(m_Value())));
2037+
EXPECT_FALSE(match(NotWithUndefs, m_NotForbidPoison(m_Value())));
20312038

20322039
Value *NotWithPoisons = IRB.CreateXor(VectorZero, VectorMixedPoison);
20332040
EXPECT_TRUE(match(NotWithPoisons, m_Not(m_Value())));
2041+
EXPECT_FALSE(match(NotWithPoisons, m_NotForbidPoison(m_Value())));
20342042

20352043
Value *NotWithUndefsCommute = IRB.CreateXor(VectorMixedUndef, VectorZero);
20362044
EXPECT_FALSE(match(NotWithUndefsCommute, m_Not(m_Value())));
2045+
EXPECT_FALSE(match(NotWithUndefsCommute, m_NotForbidPoison(m_Value())));
20372046

20382047
Value *NotWithPoisonsCommute = IRB.CreateXor(VectorMixedPoison, VectorZero);
20392048
EXPECT_TRUE(match(NotWithPoisonsCommute, m_Not(m_Value())));
2049+
EXPECT_FALSE(match(NotWithPoisonsCommute, m_NotForbidPoison(m_Value())));
20402050
}
20412051

20422052
template <typename T> struct MutableConstTest : PatternMatchTest { };

0 commit comments

Comments
 (0)