Skip to content

Commit ec0b406

Browse files
committed
[InstCombine] use logical-or matcher to avoid crash
This should prevent crashing for the example in issue #58552 by not matching a select-of-vectors-with-scalar-condition. A similar change is likely needed for the related fold to properly fix that kind of bug. The test that shows a regression seems unlikely to occur in real code. This also picks up an optimization in the case where a real (bitwise) logic op is used. We could already convert some similar select ops to real logic via impliesPoison(), so we don't see more diffs on commuted tests. Using commutative matchers (when safe) might also handle one of the TODO tests.
1 parent 29661fe commit ec0b406

File tree

2 files changed

+30
-8
lines changed

2 files changed

+30
-8
lines changed

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2769,16 +2769,16 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
27692769
return replaceInstUsesWith(SI, V);
27702770
}
27712771

2772-
// select (select a, true, b), c, false -> select a, c, false
2773-
// select c, (select a, true, b), false -> select c, a, false
2772+
// select (a || b), c, false -> select a, c, false
2773+
// select c, (a || b), false -> select c, a, false
27742774
// if c implies that b is false.
2775-
if (match(CondVal, m_Select(m_Value(A), m_One(), m_Value(B))) &&
2775+
if (match(CondVal, m_LogicalOr(m_Value(A), m_Value(B))) &&
27762776
match(FalseVal, m_Zero())) {
27772777
Optional<bool> Res = isImpliedCondition(TrueVal, B, DL);
27782778
if (Res && *Res == false)
27792779
return replaceOperand(SI, 0, A);
27802780
}
2781-
if (match(TrueVal, m_Select(m_Value(A), m_One(), m_Value(B))) &&
2781+
if (match(TrueVal, m_LogicalOr(m_Value(A), m_Value(B))) &&
27822782
match(FalseVal, m_Zero())) {
27832783
Optional<bool> Res = isImpliedCondition(CondVal, B, DL);
27842784
if (Res && *Res == false)
@@ -2787,6 +2787,7 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
27872787
// select c, true, (select a, b, false) -> select c, true, a
27882788
// select (select a, b, false), true, c -> select a, true, c
27892789
// if c = false implies that b = true
2790+
// FIXME: This should use m_LogicalAnd instead of matching a select operand.
27902791
if (match(TrueVal, m_One()) &&
27912792
match(FalseVal, m_Select(m_Value(A), m_Value(B), m_Zero()))) {
27922793
Optional<bool> Res = isImpliedCondition(CondVal, B, DL, false);

llvm/test/Transforms/InstCombine/select-safe-transforms.ll

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,8 @@ define i1 @and_orn_cmp_1_logical(i32 %a, i32 %b, i1 %y) {
144144
ret i1 %and
145145
}
146146

147+
; TODO: This should fold the same way as the next test.
148+
147149
define i1 @and_orn_cmp_1_partial_logical(i32 %a, i32 %b, i1 %y) {
148150
; CHECK-LABEL: @and_orn_cmp_1_partial_logical(
149151
; CHECK-NEXT: [[X:%.*]] = icmp sgt i32 [[A:%.*]], [[B:%.*]]
@@ -163,9 +165,7 @@ define i1 @and_orn_cmp_1_partial_logical_commute(i32 %a, i32 %b) {
163165
; CHECK-LABEL: @and_orn_cmp_1_partial_logical_commute(
164166
; CHECK-NEXT: [[Y:%.*]] = call i1 @gen1()
165167
; CHECK-NEXT: [[X:%.*]] = icmp sgt i32 [[A:%.*]], [[B:%.*]]
166-
; CHECK-NEXT: [[X_INV:%.*]] = icmp sle i32 [[A]], [[B]]
167-
; CHECK-NEXT: [[OR:%.*]] = or i1 [[Y]], [[X_INV]]
168-
; CHECK-NEXT: [[AND:%.*]] = select i1 [[X]], i1 [[OR]], i1 false
168+
; CHECK-NEXT: [[AND:%.*]] = select i1 [[X]], i1 [[Y]], i1 false
169169
; CHECK-NEXT: ret i1 [[AND]]
170170
;
171171
%y = call i1 @gen1() ; thwart complexity-based canonicalization
@@ -219,10 +219,31 @@ define i1 @andn_or_cmp_2_partial_logical_commute(i16 %a, i16 %b) {
219219
ret i1 %and
220220
}
221221

222+
; PR58552 - this would crash trying to replace non-matching types
223+
224+
define <2 x i1> @not_logical_or(i1 %b, <2 x i32> %a) {
225+
; CHECK-LABEL: @not_logical_or(
226+
; CHECK-NEXT: [[COND:%.*]] = icmp ult <2 x i32> [[A:%.*]], <i32 3, i32 3>
227+
; CHECK-NEXT: [[IMPLIED:%.*]] = icmp slt <2 x i32> [[A]], <i32 -1, i32 -1>
228+
; CHECK-NEXT: [[OR:%.*]] = select i1 [[B:%.*]], <2 x i1> <i1 true, i1 true>, <2 x i1> [[IMPLIED]]
229+
; CHECK-NEXT: [[AND:%.*]] = select <2 x i1> [[COND]], <2 x i1> [[OR]], <2 x i1> zeroinitializer
230+
; CHECK-NEXT: ret <2 x i1> [[AND]]
231+
;
232+
%cond = icmp ult <2 x i32> %a, <i32 3, i32 3>
233+
%implied = icmp slt <2 x i32> %a, <i32 -1, i32 -1>
234+
%or = select i1 %b, <2 x i1> <i1 true, i1 true>, <2 x i1> %implied
235+
%and = select <2 x i1> %cond, <2 x i1> %or, <2 x i1> zeroinitializer
236+
ret <2 x i1> %and
237+
}
238+
239+
; This could reduce, but we do not match select-of-vectors with scalar condition as logical-or.
240+
222241
define <2 x i1> @not_logical_or2(i1 %b, <2 x i32> %a) {
223242
; CHECK-LABEL: @not_logical_or2(
224243
; CHECK-NEXT: [[COND:%.*]] = icmp ult <2 x i32> [[A:%.*]], <i32 3, i32 3>
225-
; CHECK-NEXT: [[AND:%.*]] = select i1 [[B:%.*]], <2 x i1> [[COND]], <2 x i1> zeroinitializer
244+
; CHECK-NEXT: [[IMPLIED:%.*]] = icmp slt <2 x i32> [[A]], <i32 -1, i32 -1>
245+
; CHECK-NEXT: [[OR:%.*]] = select i1 [[B:%.*]], <2 x i1> <i1 true, i1 true>, <2 x i1> [[IMPLIED]]
246+
; CHECK-NEXT: [[AND:%.*]] = select <2 x i1> [[OR]], <2 x i1> [[COND]], <2 x i1> zeroinitializer
226247
; CHECK-NEXT: ret <2 x i1> [[AND]]
227248
;
228249
%cond = icmp ult <2 x i32> %a, <i32 3, i32 3>

0 commit comments

Comments
 (0)