Skip to content

Commit 9ef8290

Browse files
committed
[InstCombine] Fix buggy transform in foldNestedSelects; PR 71330
The bug is that `IsAndVariant` is used to assume which arm in the select the output `SelInner` should be placed but match the inner select condition with `m_c_LogicalOp`. With fully simplified ops, this works fine, but its possible if the select condition is not simplified, for it match both `LogicalAnd` and `LogicalOr` i.e `select true, true, false`. In PR71330 for example, the issue occurs in the following IR: ``` define i32 @bad() { %..i.i = select i1 false, i32 0, i32 3 %brmerge = select i1 true, i1 true, i1 false %not.cmp.i.i.not = xor i1 true, true %.mux = zext i1 %not.cmp.i.i.not to i32 %retval.0.i.i = select i1 %brmerge, i32 %.mux, i32 %..i.i ret i32 %retval.0.i.i } ``` When simplifying: ``` %retval.0.i.i = select i1 %brmerge, i32 %.mux, i32 %..i.i ``` We end up matching `%brmerge` as `LogicalAnd` for `IsAndVariant`, but the inner select (`%..i.i`) condition which is `false` with `LogicalOr`. Closes #71489
1 parent 5cc9347 commit 9ef8290

File tree

2 files changed

+10
-4
lines changed

2 files changed

+10
-4
lines changed

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2891,8 +2891,15 @@ static Instruction *foldNestedSelects(SelectInst &OuterSelVal,
28912891
std::swap(InnerSel.TrueVal, InnerSel.FalseVal);
28922892

28932893
Value *AltCond = nullptr;
2894-
auto matchOuterCond = [OuterSel, &AltCond](auto m_InnerCond) {
2895-
return match(OuterSel.Cond, m_c_LogicalOp(m_InnerCond, m_Value(AltCond)));
2894+
auto matchOuterCond = [OuterSel, IsAndVariant, &AltCond](auto m_InnerCond) {
2895+
// An unsimplified select condition can match both LogicalAnd and LogicalOr
2896+
// (select true, true, false). Since below we assume that LogicalAnd implies
2897+
// InnerSel match the FVal and vice versa for LogicalOr, we can't match the
2898+
// alternative pattern here.
2899+
return IsAndVariant ? match(OuterSel.Cond,
2900+
m_c_LogicalAnd(m_InnerCond, m_Value(AltCond)))
2901+
: match(OuterSel.Cond,
2902+
m_c_LogicalOr(m_InnerCond, m_Value(AltCond)));
28962903
};
28972904

28982905
// Finally, match the condition that was driving the outermost `select`,

llvm/test/Transforms/InstCombine/pr71330.ll

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ define void @pr71330(i32 %conv, i1 %tobool19.not4, i16 %lb) {
1515
; CHECK: for.cond7.preheader.split.us.split:
1616
; CHECK-NEXT: ret void
1717
; CHECK: for.cond7:
18-
; CHECK-NEXT: [[ADD9:%.*]] = add i32 [[CONV]], 3
19-
; CHECK-NEXT: [[CMP12:%.*]] = icmp slt i32 [[ADD9]], 0
18+
; CHECK-NEXT: [[CMP12:%.*]] = icmp slt i32 [[CONV]], 0
2019
; CHECK-NEXT: br i1 [[CMP12]], label [[FOR_BODY14:%.*]], label [[FOR_END25]]
2120
; CHECK: for.body14:
2221
; CHECK-NEXT: ret void

0 commit comments

Comments
 (0)