Skip to content

Commit

Permalink
[InstCombine] use logical-or matcher to avoid crash
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rotateright committed Nov 1, 2022
1 parent 29661fe commit ec0b406
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 8 deletions.
9 changes: 5 additions & 4 deletions llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
Expand Up @@ -2769,16 +2769,16 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
return replaceInstUsesWith(SI, V);
}

// select (select a, true, b), c, false -> select a, c, false
// select c, (select a, true, b), false -> select c, a, false
// select (a || b), c, false -> select a, c, false
// select c, (a || b), false -> select c, a, false
// if c implies that b is false.
if (match(CondVal, m_Select(m_Value(A), m_One(), m_Value(B))) &&
if (match(CondVal, m_LogicalOr(m_Value(A), m_Value(B))) &&
match(FalseVal, m_Zero())) {
Optional<bool> Res = isImpliedCondition(TrueVal, B, DL);
if (Res && *Res == false)
return replaceOperand(SI, 0, A);
}
if (match(TrueVal, m_Select(m_Value(A), m_One(), m_Value(B))) &&
if (match(TrueVal, m_LogicalOr(m_Value(A), m_Value(B))) &&
match(FalseVal, m_Zero())) {
Optional<bool> Res = isImpliedCondition(CondVal, B, DL);
if (Res && *Res == false)
Expand All @@ -2787,6 +2787,7 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
// select c, true, (select a, b, false) -> select c, true, a
// select (select a, b, false), true, c -> select a, true, c
// if c = false implies that b = true
// FIXME: This should use m_LogicalAnd instead of matching a select operand.
if (match(TrueVal, m_One()) &&
match(FalseVal, m_Select(m_Value(A), m_Value(B), m_Zero()))) {
Optional<bool> Res = isImpliedCondition(CondVal, B, DL, false);
Expand Down
29 changes: 25 additions & 4 deletions llvm/test/Transforms/InstCombine/select-safe-transforms.ll
Expand Up @@ -144,6 +144,8 @@ define i1 @and_orn_cmp_1_logical(i32 %a, i32 %b, i1 %y) {
ret i1 %and
}

; TODO: This should fold the same way as the next test.

define i1 @and_orn_cmp_1_partial_logical(i32 %a, i32 %b, i1 %y) {
; CHECK-LABEL: @and_orn_cmp_1_partial_logical(
; CHECK-NEXT: [[X:%.*]] = icmp sgt i32 [[A:%.*]], [[B:%.*]]
Expand All @@ -163,9 +165,7 @@ define i1 @and_orn_cmp_1_partial_logical_commute(i32 %a, i32 %b) {
; CHECK-LABEL: @and_orn_cmp_1_partial_logical_commute(
; CHECK-NEXT: [[Y:%.*]] = call i1 @gen1()
; CHECK-NEXT: [[X:%.*]] = icmp sgt i32 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[X_INV:%.*]] = icmp sle i32 [[A]], [[B]]
; CHECK-NEXT: [[OR:%.*]] = or i1 [[Y]], [[X_INV]]
; CHECK-NEXT: [[AND:%.*]] = select i1 [[X]], i1 [[OR]], i1 false
; CHECK-NEXT: [[AND:%.*]] = select i1 [[X]], i1 [[Y]], i1 false
; CHECK-NEXT: ret i1 [[AND]]
;
%y = call i1 @gen1() ; thwart complexity-based canonicalization
Expand Down Expand Up @@ -219,10 +219,31 @@ define i1 @andn_or_cmp_2_partial_logical_commute(i16 %a, i16 %b) {
ret i1 %and
}

; PR58552 - this would crash trying to replace non-matching types

define <2 x i1> @not_logical_or(i1 %b, <2 x i32> %a) {
; CHECK-LABEL: @not_logical_or(
; CHECK-NEXT: [[COND:%.*]] = icmp ult <2 x i32> [[A:%.*]], <i32 3, i32 3>
; CHECK-NEXT: [[IMPLIED:%.*]] = icmp slt <2 x i32> [[A]], <i32 -1, i32 -1>
; CHECK-NEXT: [[OR:%.*]] = select i1 [[B:%.*]], <2 x i1> <i1 true, i1 true>, <2 x i1> [[IMPLIED]]
; CHECK-NEXT: [[AND:%.*]] = select <2 x i1> [[COND]], <2 x i1> [[OR]], <2 x i1> zeroinitializer
; CHECK-NEXT: ret <2 x i1> [[AND]]
;
%cond = icmp ult <2 x i32> %a, <i32 3, i32 3>
%implied = icmp slt <2 x i32> %a, <i32 -1, i32 -1>
%or = select i1 %b, <2 x i1> <i1 true, i1 true>, <2 x i1> %implied
%and = select <2 x i1> %cond, <2 x i1> %or, <2 x i1> zeroinitializer
ret <2 x i1> %and
}

; This could reduce, but we do not match select-of-vectors with scalar condition as logical-or.

define <2 x i1> @not_logical_or2(i1 %b, <2 x i32> %a) {
; CHECK-LABEL: @not_logical_or2(
; CHECK-NEXT: [[COND:%.*]] = icmp ult <2 x i32> [[A:%.*]], <i32 3, i32 3>
; CHECK-NEXT: [[AND:%.*]] = select i1 [[B:%.*]], <2 x i1> [[COND]], <2 x i1> zeroinitializer
; CHECK-NEXT: [[IMPLIED:%.*]] = icmp slt <2 x i32> [[A]], <i32 -1, i32 -1>
; CHECK-NEXT: [[OR:%.*]] = select i1 [[B:%.*]], <2 x i1> <i1 true, i1 true>, <2 x i1> [[IMPLIED]]
; CHECK-NEXT: [[AND:%.*]] = select <2 x i1> [[OR]], <2 x i1> [[COND]], <2 x i1> zeroinitializer
; CHECK-NEXT: ret <2 x i1> [[AND]]
;
%cond = icmp ult <2 x i32> %a, <i32 3, i32 3>
Expand Down

0 comments on commit ec0b406

Please sign in to comment.