Skip to content

Commit b24e2f6

Browse files
committed
[InstCombine] use logical-and matcher to avoid crash
Follow-on to: ec0b406 This should prevent crashing for example like issue #58552 by not matching a select-of-vectors-with-scalar-condition. 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 6f77979 commit b24e2f6

File tree

2 files changed

+31
-11
lines changed

2 files changed

+31
-11
lines changed

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2784,17 +2784,16 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
27842784
if (Res && *Res == false)
27852785
return replaceOperand(SI, 1, A);
27862786
}
2787-
// select c, true, (select a, b, false) -> select c, true, a
2788-
// select (select a, b, false), true, c -> select a, true, c
2787+
// select c, true, (a && b) -> select c, true, a
2788+
// select (a && b), 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.
27912790
if (match(TrueVal, m_One()) &&
2792-
match(FalseVal, m_Select(m_Value(A), m_Value(B), m_Zero()))) {
2791+
match(FalseVal, m_LogicalAnd(m_Value(A), m_Value(B)))) {
27932792
Optional<bool> Res = isImpliedCondition(CondVal, B, DL, false);
27942793
if (Res && *Res == true)
27952794
return replaceOperand(SI, 2, A);
27962795
}
2797-
if (match(CondVal, m_Select(m_Value(A), m_Value(B), m_Zero())) &&
2796+
if (match(CondVal, m_LogicalAnd(m_Value(A), m_Value(B))) &&
27982797
match(TrueVal, m_One())) {
27992798
Optional<bool> Res = isImpliedCondition(FalseVal, B, DL, false);
28002799
if (Res && *Res == true)

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

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,8 @@ define i1 @orn_and_cmp_1_logical(i37 %a, i37 %b, i1 %y) {
685685
ret i1 %or
686686
}
687687

688+
; TODO: This should fold the same way as the next test.
689+
688690
define i1 @orn_and_cmp_1_partial_logical(i37 %a, i37 %b, i1 %y) {
689691
; CHECK-LABEL: @orn_and_cmp_1_partial_logical(
690692
; CHECK-NEXT: [[X:%.*]] = icmp sgt i37 [[A:%.*]], [[B:%.*]]
@@ -703,10 +705,8 @@ define i1 @orn_and_cmp_1_partial_logical(i37 %a, i37 %b, i1 %y) {
703705
define i1 @orn_and_cmp_1_partial_logical_commute(i37 %a, i37 %b) {
704706
; CHECK-LABEL: @orn_and_cmp_1_partial_logical_commute(
705707
; CHECK-NEXT: [[Y:%.*]] = call i1 @gen1()
706-
; CHECK-NEXT: [[X:%.*]] = icmp sgt i37 [[A:%.*]], [[B:%.*]]
707-
; CHECK-NEXT: [[X_INV:%.*]] = icmp sle i37 [[A]], [[B]]
708-
; CHECK-NEXT: [[AND:%.*]] = and i1 [[Y]], [[X]]
709-
; CHECK-NEXT: [[OR:%.*]] = select i1 [[X_INV]], i1 true, i1 [[AND]]
708+
; CHECK-NEXT: [[X_INV:%.*]] = icmp sle i37 [[A:%.*]], [[B:%.*]]
709+
; CHECK-NEXT: [[OR:%.*]] = select i1 [[X_INV]], i1 true, i1 [[Y]]
710710
; CHECK-NEXT: ret i1 [[OR]]
711711
;
712712
%y = call i1 @gen1() ; thwart complexity-based canonicalization
@@ -760,10 +760,31 @@ define i1 @orn_and_cmp_2_partial_logical_commute(i16 %a, i16 %b) {
760760
ret i1 %or
761761
}
762762

763+
; PR58552 - this would crash trying to replace non-matching types
764+
765+
define <2 x i1> @not_logical_and(i1 %b, <2 x i32> %a) {
766+
; CHECK-LABEL: @not_logical_and(
767+
; CHECK-NEXT: [[COND:%.*]] = icmp ult <2 x i32> [[A:%.*]], <i32 3, i32 3>
768+
; CHECK-NEXT: [[IMPLIED:%.*]] = icmp ugt <2 x i32> [[A]], <i32 1, i32 1>
769+
; CHECK-NEXT: [[AND:%.*]] = select i1 [[B:%.*]], <2 x i1> [[COND]], <2 x i1> zeroinitializer
770+
; CHECK-NEXT: [[OR:%.*]] = select <2 x i1> [[IMPLIED]], <2 x i1> <i1 true, i1 true>, <2 x i1> [[AND]]
771+
; CHECK-NEXT: ret <2 x i1> [[OR]]
772+
;
773+
%cond = icmp ult <2 x i32> %a, <i32 3, i32 3>
774+
%implied = icmp ugt <2 x i32> %a, <i32 1, i32 1>
775+
%and = select i1 %b, <2 x i1> %cond, <2 x i1> zeroinitializer
776+
%or = select <2 x i1> %implied, <2 x i1> <i1 true, i1 true>, <2 x i1> %and
777+
ret <2 x i1> %or
778+
}
779+
780+
; This could reduce, but we do not match select-of-vectors with scalar condition as logical-and.
781+
763782
define <2 x i1> @not_logical_and2(i1 %b, <2 x i32> %a) {
764783
; CHECK-LABEL: @not_logical_and2(
765-
; CHECK-NEXT: [[IMPLIED:%.*]] = icmp ugt <2 x i32> [[A:%.*]], <i32 1, i32 1>
766-
; CHECK-NEXT: [[OR:%.*]] = select i1 [[B:%.*]], <2 x i1> <i1 true, i1 true>, <2 x i1> [[IMPLIED]]
784+
; CHECK-NEXT: [[COND:%.*]] = icmp ult <2 x i32> [[A:%.*]], <i32 3, i32 3>
785+
; CHECK-NEXT: [[IMPLIED:%.*]] = icmp ugt <2 x i32> [[A]], <i32 1, i32 1>
786+
; CHECK-NEXT: [[AND:%.*]] = select i1 [[B:%.*]], <2 x i1> [[COND]], <2 x i1> zeroinitializer
787+
; CHECK-NEXT: [[OR:%.*]] = select <2 x i1> [[AND]], <2 x i1> <i1 true, i1 true>, <2 x i1> [[IMPLIED]]
767788
; CHECK-NEXT: ret <2 x i1> [[OR]]
768789
;
769790
%cond = icmp ult <2 x i32> %a, <i32 3, i32 3>

0 commit comments

Comments
 (0)