[llvm] b24e2f6 - [InstCombine] use logical-and matcher to avoid crash

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2022-11-02T08:23:52-04:00
New Revision: b24e2f6ef6704652f52e1dc24e4e1cae5144fb7a

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

LOG: [InstCombine] use logical-and matcher to avoid crash

Follow-on to:
ec0b406e16c44f1554

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.

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 5df7459e4985..f4ad343a614e 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -2784,17 +2784,16 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
     if (Res && *Res == false)
       return replaceOperand(SI, 1, A);
   }
-  // select c, true, (select a, b, false)  -> select c, true, a
-  // select (select a, b, false), true, c  -> select a, true, c
+  // select c, true, (a && b)  -> select c, true, a
+  // select (a && b), 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()))) {
+      match(FalseVal, m_LogicalAnd(m_Value(A), m_Value(B)))) {
     Optional<bool> Res = isImpliedCondition(CondVal, B, DL, false);
     if (Res && *Res == true)
       return replaceOperand(SI, 2, A);
   }
-  if (match(CondVal, m_Select(m_Value(A), m_Value(B), m_Zero())) &&
+  if (match(CondVal, m_LogicalAnd(m_Value(A), m_Value(B))) &&
       match(TrueVal, m_One())) {
     Optional<bool> Res = isImpliedCondition(FalseVal, B, DL, false);
     if (Res && *Res == true)

diff  --git a/llvm/test/Transforms/InstCombine/select-safe-transforms.ll b/llvm/test/Transforms/InstCombine/select-safe-transforms.ll
index d34193bbe31e..e5c313c361d5 100644
--- a/llvm/test/Transforms/InstCombine/select-safe-transforms.ll
+++ b/llvm/test/Transforms/InstCombine/select-safe-transforms.ll
@@ -685,6 +685,8 @@ define i1 @orn_and_cmp_1_logical(i37 %a, i37 %b, i1 %y) {
   ret i1 %or
 }
 
+; TODO: This should fold the same way as the next test.
+
 define i1 @orn_and_cmp_1_partial_logical(i37 %a, i37 %b, i1 %y) {
 ; CHECK-LABEL: @orn_and_cmp_1_partial_logical(
 ; 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) {
 define i1 @orn_and_cmp_1_partial_logical_commute(i37 %a, i37 %b) {
 ; CHECK-LABEL: @orn_and_cmp_1_partial_logical_commute(
 ; CHECK-NEXT:    [[Y:%.*]] = call i1 @gen1()
-; CHECK-NEXT:    [[X:%.*]] = icmp sgt i37 [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT:    [[X_INV:%.*]] = icmp sle i37 [[A]], [[B]]
-; CHECK-NEXT:    [[AND:%.*]] = and i1 [[Y]], [[X]]
-; CHECK-NEXT:    [[OR:%.*]] = select i1 [[X_INV]], i1 true, i1 [[AND]]
+; CHECK-NEXT:    [[X_INV:%.*]] = icmp sle i37 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[OR:%.*]] = select i1 [[X_INV]], i1 true, i1 [[Y]]
 ; CHECK-NEXT:    ret i1 [[OR]]
 ;
   %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) {
   ret i1 %or
 }
 
+; PR58552 - this would crash trying to replace non-matching types
+
+define <2 x i1> @not_logical_and(i1 %b, <2 x i32> %a) {
+; CHECK-LABEL: @not_logical_and(
+; CHECK-NEXT:    [[COND:%.*]] = icmp ult <2 x i32> [[A:%.*]], <i32 3, i32 3>
+; CHECK-NEXT:    [[IMPLIED:%.*]] = icmp ugt <2 x i32> [[A]], <i32 1, i32 1>
+; CHECK-NEXT:    [[AND:%.*]] = select i1 [[B:%.*]], <2 x i1> [[COND]], <2 x i1> zeroinitializer
+; CHECK-NEXT:    [[OR:%.*]] = select <2 x i1> [[IMPLIED]], <2 x i1> <i1 true, i1 true>, <2 x i1> [[AND]]
+; CHECK-NEXT:    ret <2 x i1> [[OR]]
+;
+  %cond = icmp ult <2 x i32> %a, <i32 3, i32 3>
+  %implied = icmp ugt <2 x i32> %a, <i32 1, i32 1>
+  %and = select i1 %b, <2 x i1> %cond, <2 x i1> zeroinitializer
+  %or = select <2 x i1> %implied, <2 x i1> <i1 true, i1 true>, <2 x i1> %and
+  ret <2 x i1> %or
+}
+
+; This could reduce, but we do not match select-of-vectors with scalar condition as logical-and.
+
 define <2 x i1> @not_logical_and2(i1 %b, <2 x i32> %a) {
 ; CHECK-LABEL: @not_logical_and2(
-; CHECK-NEXT:    [[IMPLIED:%.*]] = icmp ugt <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:    [[COND:%.*]] = icmp ult <2 x i32> [[A:%.*]], <i32 3, i32 3>
+; CHECK-NEXT:    [[IMPLIED:%.*]] = icmp ugt <2 x i32> [[A]], <i32 1, i32 1>
+; CHECK-NEXT:    [[AND:%.*]] = select i1 [[B:%.*]], <2 x i1> [[COND]], <2 x i1> zeroinitializer
+; CHECK-NEXT:    [[OR:%.*]] = select <2 x i1> [[AND]], <2 x i1> <i1 true, i1 true>, <2 x i1> [[IMPLIED]]
 ; CHECK-NEXT:    ret <2 x i1> [[OR]]
 ;
   %cond = icmp ult <2 x i32> %a, <i32 3, i32 3>