[llvm] ec0b406 - [InstCombine] use logical-or matcher to avoid crash

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2022-11-01T16:47:41-04:00
New Revision: ec0b406e16c44f1554e409b20ba6d7a76e3fe08d

URL: https://github.com/llvm/llvm-project/commit/ec0b406e16c44f1554e409b20ba6d7a76e3fe08d
DIFF: https://github.com/llvm/llvm-project/commit/ec0b406e16c44f1554e409b20ba6d7a76e3fe08d.diff

LOG: [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.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
    llvm/test/Transforms/InstCombine/select-safe-transforms.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index f5bd2936606ce..5df7459e49851 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -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)
@@ -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);

diff  --git a/llvm/test/Transforms/InstCombine/select-safe-transforms.ll b/llvm/test/Transforms/InstCombine/select-safe-transforms.ll
index 7263edcf60ac5..dba59931235de 100644
--- a/llvm/test/Transforms/InstCombine/select-safe-transforms.ll
+++ b/llvm/test/Transforms/InstCombine/select-safe-transforms.ll
@@ -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:%.*]]
@@ -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
@@ -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>