[llvm] [InstCombine] Extend Phi-Icmp use to include or (PR #67682)

via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 5 06:00:00 PDT 2023


https://github.com/bipmis updated https://github.com/llvm/llvm-project/pull/67682

>From ec137d77bff23eee2fcb65ec8970f2190d2b8002 Mon Sep 17 00:00:00 2001
From: bipmis <biplob.mishra at arm.com>
Date: Thu, 28 Sep 2023 14:48:11 +0100
Subject: [PATCH 1/4] Extend PhiIcmp use to include or

---
 .../Transforms/InstCombine/InstCombinePHI.cpp |  72 +++++++----
 llvm/test/Transforms/InstCombine/phi.ll       | 121 ++++++++++++++++++
 2 files changed, 167 insertions(+), 26 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index 00115abf6500597..464c2848b6c3554 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -1441,36 +1441,56 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) {
         PHIUser->user_back() == &PN) {
       return replaceInstUsesWith(PN, PoisonValue::get(PN.getType()));
     }
-    // When a PHI is used only to be compared with zero, it is safe to replace
-    // an incoming value proved as known nonzero with any non-zero constant.
-    // For example, in the code below, the incoming value %v can be replaced
-    // with any non-zero constant based on the fact that the PHI is only used to
-    // be compared with zero and %v is a known non-zero value:
-    // %v = select %cond, 1, 2
-    // %p = phi [%v, BB] ...
-    //      icmp eq, %p, 0
-    auto *CmpInst = dyn_cast<ICmpInst>(PHIUser);
-    // FIXME: To be simple, handle only integer type for now.
-    if (CmpInst && isa<IntegerType>(PN.getType()) && CmpInst->isEquality() &&
-        match(CmpInst->getOperand(1), m_Zero())) {
-      ConstantInt *NonZeroConst = nullptr;
-      bool MadeChange = false;
-      for (unsigned I = 0, E = PN.getNumIncomingValues(); I != E; ++I) {
-        Instruction *CtxI = PN.getIncomingBlock(I)->getTerminator();
-        Value *VA = PN.getIncomingValue(I);
-        if (isKnownNonZero(VA, DL, 0, &AC, CtxI, &DT)) {
-          if (!NonZeroConst)
-            NonZeroConst = getAnyNonZeroConstInt(PN);
+  }
 
-          if (NonZeroConst != VA) {
-            replaceOperand(PN, I, NonZeroConst);
-            MadeChange = true;
-          }
+  // When a PHI is used only to be compared with zero, it is safe to replace
+  // an incoming value proved as known nonzero with any non-zero constant.
+  // For example, in the code below, the incoming value %v can be replaced
+  // with any non-zero constant based on the fact that the PHI is only used to
+  // be compared with zero and %v is a known non-zero value:
+  // %v = select %cond, 1, 2
+  // %p = phi [%v, BB] ...
+  //      icmp eq, %p, 0
+  // FIXME: To be simple, handle only integer type for now.
+  // Extend to 2 use of phi -> icmp and or(icmp)
+  bool AllUsesOfPhiEndsInCmp = false;
+  if (PN.hasOneUse() || PN.hasNUses(2)) {
+    for (const auto *U : PN.users()) {
+      auto *CmpInst = dyn_cast<ICmpInst>(U);
+      if (!CmpInst) {
+        // This is always correct as OR only add bits and we are checking
+        // against 0.
+        if (U->hasOneUse() && match(U, m_Or(m_Specific(&PN), m_Value())))
+          CmpInst = dyn_cast<ICmpInst>(U->user_back());
+      }
+      if (CmpInst && isa<IntegerType>(PN.getType()) && CmpInst->isEquality() &&
+          match(CmpInst->getOperand(1), m_Zero()))
+        AllUsesOfPhiEndsInCmp = true;
+      else {
+        AllUsesOfPhiEndsInCmp = false;
+        break;
+      }
+    }
+  }
+
+  // All uses of PHI results in a compare with zero.
+  if (AllUsesOfPhiEndsInCmp) {
+    ConstantInt *NonZeroConst = nullptr;
+    bool MadeChange = false;
+    for (unsigned I = 0, E = PN.getNumIncomingValues(); I != E; ++I) {
+      Instruction *CtxI = PN.getIncomingBlock(I)->getTerminator();
+      Value *VA = PN.getIncomingValue(I);
+      if (isKnownNonZero(VA, DL, 0, &AC, CtxI, &DT)) {
+        if (!NonZeroConst)
+          NonZeroConst = getAnyNonZeroConstInt(PN);
+        if (NonZeroConst != VA) {
+          replaceOperand(PN, I, NonZeroConst);
+          MadeChange = true;
         }
       }
-      if (MadeChange)
-        return &PN;
     }
+    if (MadeChange)
+      return &PN;
   }
 
   // We sometimes end up with phi cycles that non-obviously end up being the
diff --git a/llvm/test/Transforms/InstCombine/phi.ll b/llvm/test/Transforms/InstCombine/phi.ll
index a5cab8a71d3f711..3c0d4c9b055bcce 100644
--- a/llvm/test/Transforms/InstCombine/phi.ll
+++ b/llvm/test/Transforms/InstCombine/phi.ll
@@ -1286,6 +1286,127 @@ if.end:                                           ; preds = %entry, %if.then
   ret i1  %cmp1
 }
 
+define i1 @phi_knownnonzero_eq_oricmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
+; CHECK-LABEL: @phi_knownnonzero_eq_oricmp(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp slt i32 [[N:%.*]], [[S:%.*]]
+; CHECK-NEXT:    br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    [[A_0:%.*]] = phi i32 [ 1, [[IF_THEN]] ], [ [[N]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[TMP0:%.*]] = or i32 [[A_0]], [[VAL:%.*]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq i32 [[TMP0]], 0
+; CHECK-NEXT:    ret i1 [[CMP1]]
+;
+entry:
+  %tobool = icmp slt  i32 %n, %s
+  br i1 %tobool, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = load i32, ptr %P
+  %cmp = icmp eq i32 %n, %0
+  %1 = select i1 %cmp, i32 1, i32 2
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.then
+  %a.0 = phi i32 [ %1,  %if.then ], [ %n, %entry ]
+  %2 = or i32 %a.0, %val
+  %cmp1 = icmp eq i32 %2, 0
+  ret i1  %cmp1
+}
+
+define i1 @phi_knownnonzero_eq_multiuse_oricmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
+; CHECK-LABEL: @phi_knownnonzero_eq_multiuse_oricmp(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp slt i32 [[N:%.*]], [[S:%.*]]
+; CHECK-NEXT:    br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    [[A_0:%.*]] = phi i32 [ 1, [[IF_THEN]] ], [ [[N]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[TMP0:%.*]] = or i32 [[A_0]], [[VAL:%.*]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq i32 [[TMP0]], 0
+; CHECK-NEXT:    br i1 [[CMP1]], label [[NEXT:%.*]], label [[CLEANUP:%.*]]
+; CHECK:       next:
+; CHECK-NEXT:    [[BOOL2:%.*]] = icmp eq i32 [[A_0]], 0
+; CHECK-NEXT:    br label [[CLEANUP]]
+; CHECK:       cleanup:
+; CHECK-NEXT:    [[FINAL:%.*]] = phi i1 [ [[CMP1]], [[IF_END]] ], [ [[BOOL2]], [[NEXT]] ]
+; CHECK-NEXT:    ret i1 [[FINAL]]
+;
+entry:
+  %tobool = icmp slt  i32 %n, %s
+  br i1 %tobool, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = load i32, ptr %P
+  %cmp = icmp eq i32 %n, %0
+  %1 = select i1 %cmp, i32 1, i32 2
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.then
+  %a.0 = phi i32 [ %1,  %if.then ], [ %n, %entry ]
+  %2 = or i32 %a.0, %val
+  %cmp1 = icmp eq i32 %2, 0
+  br i1 %cmp1, label %next, label %cleanup
+
+next:
+  %bool2 = icmp eq i32 %a.0, 0
+  br label %cleanup
+
+cleanup:
+  %final =  phi i1 [ %cmp1,  %if.end ], [ %bool2, %next ]
+  ret i1  %final
+}
+
+define i1 @phi_knownnonzero_eq_multiuse_andicmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
+; CHECK-LABEL: @phi_knownnonzero_eq_multiuse_andicmp(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp slt i32 [[N:%.*]], [[S:%.*]]
+; CHECK-NEXT:    br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr [[P:%.*]], align 4
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[N]]
+; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[CMP]], i32 1, i32 2
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    [[A_0:%.*]] = phi i32 [ [[TMP1]], [[IF_THEN]] ], [ [[N]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[TMP2:%.*]] = and i32 [[A_0]], [[VAL:%.*]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq i32 [[TMP2]], 0
+; CHECK-NEXT:    br i1 [[CMP1]], label [[NEXT:%.*]], label [[CLEANUP:%.*]]
+; CHECK:       next:
+; CHECK-NEXT:    [[BOOL2:%.*]] = icmp eq i32 [[A_0]], 0
+; CHECK-NEXT:    br label [[CLEANUP]]
+; CHECK:       cleanup:
+; CHECK-NEXT:    [[FINAL:%.*]] = phi i1 [ [[CMP1]], [[IF_END]] ], [ [[BOOL2]], [[NEXT]] ]
+; CHECK-NEXT:    ret i1 [[FINAL]]
+;
+entry:
+  %tobool = icmp slt  i32 %n, %s
+  br i1 %tobool, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = load i32, ptr %P
+  %cmp = icmp eq i32 %n, %0
+  %1 = select i1 %cmp, i32 1, i32 2
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.then
+  %a.0 = phi i32 [ %1,  %if.then ], [ %n, %entry ]
+  %2 = and i32 %a.0, %val
+  %cmp1 = icmp eq i32 %2, 0
+  br i1 %cmp1, label %next, label %cleanup
+
+next:
+  %bool2 = icmp eq i32 %a.0, 0
+  br label %cleanup
+
+cleanup:
+  %final =  phi i1 [ %cmp1,  %if.end ], [ %bool2, %next ]
+  ret i1  %final
+}
+
 ; This would crash trying to delete an instruction (conv)
 ; that still had uses because the user (the phi) was not
 ; updated to remove a use from an unreachable block (g.exit).

>From 22555443e151c44fe06faf2e8e3327900f109d75 Mon Sep 17 00:00:00 2001
From: bipmis <biplob.mishra at arm.com>
Date: Mon, 2 Oct 2023 18:59:04 +0100
Subject: [PATCH 2/4] Address Review Comments and add icmp ne tests

---
 .../Transforms/InstCombine/InstCombinePHI.cpp |  45 ++++---
 llvm/test/Transforms/InstCombine/phi.ll       | 121 ++++++++++++++++++
 2 files changed, 143 insertions(+), 23 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index 464c2848b6c3554..db21c7da22d600d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -1452,9 +1452,11 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) {
   // %p = phi [%v, BB] ...
   //      icmp eq, %p, 0
   // FIXME: To be simple, handle only integer type for now.
-  // Extend to 2 use of phi -> icmp and or(icmp)
-  bool AllUsesOfPhiEndsInCmp = false;
+  // Extend to 2 use of phi -> icmp and or(icmp). Putting a limit on number of
+  // uses as it turns a O(1) algorithm into a O(n) algorithm. And if we are
+  // doing it for each instruction it turns into O(n^2).
   if (PN.hasOneUse() || PN.hasNUses(2)) {
+    bool AllUsesOfPhiEndsInCmp = true;
     for (const auto *U : PN.users()) {
       auto *CmpInst = dyn_cast<ICmpInst>(U);
       if (!CmpInst) {
@@ -1463,34 +1465,31 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) {
         if (U->hasOneUse() && match(U, m_Or(m_Specific(&PN), m_Value())))
           CmpInst = dyn_cast<ICmpInst>(U->user_back());
       }
-      if (CmpInst && isa<IntegerType>(PN.getType()) && CmpInst->isEquality() &&
-          match(CmpInst->getOperand(1), m_Zero()))
-        AllUsesOfPhiEndsInCmp = true;
-      else {
+      if (!CmpInst || !isa<IntegerType>(PN.getType()) ||
+          !CmpInst->isEquality() || !match(CmpInst->getOperand(1), m_Zero())) {
         AllUsesOfPhiEndsInCmp = false;
         break;
       }
     }
-  }
-
-  // All uses of PHI results in a compare with zero.
-  if (AllUsesOfPhiEndsInCmp) {
-    ConstantInt *NonZeroConst = nullptr;
-    bool MadeChange = false;
-    for (unsigned I = 0, E = PN.getNumIncomingValues(); I != E; ++I) {
-      Instruction *CtxI = PN.getIncomingBlock(I)->getTerminator();
-      Value *VA = PN.getIncomingValue(I);
-      if (isKnownNonZero(VA, DL, 0, &AC, CtxI, &DT)) {
-        if (!NonZeroConst)
-          NonZeroConst = getAnyNonZeroConstInt(PN);
-        if (NonZeroConst != VA) {
-          replaceOperand(PN, I, NonZeroConst);
-          MadeChange = true;
+    // All uses of PHI results in a compare with zero.
+    if (AllUsesOfPhiEndsInCmp) {
+      ConstantInt *NonZeroConst = nullptr;
+      bool MadeChange = false;
+      for (unsigned I = 0, E = PN.getNumIncomingValues(); I != E; ++I) {
+        Instruction *CtxI = PN.getIncomingBlock(I)->getTerminator();
+        Value *VA = PN.getIncomingValue(I);
+        if (isKnownNonZero(VA, DL, 0, &AC, CtxI, &DT)) {
+          if (!NonZeroConst)
+            NonZeroConst = getAnyNonZeroConstInt(PN);
+          if (NonZeroConst != VA) {
+            replaceOperand(PN, I, NonZeroConst);
+            MadeChange = true;
+          }
         }
       }
+      if (MadeChange)
+        return &PN;
     }
-    if (MadeChange)
-      return &PN;
   }
 
   // We sometimes end up with phi cycles that non-obviously end up being the
diff --git a/llvm/test/Transforms/InstCombine/phi.ll b/llvm/test/Transforms/InstCombine/phi.ll
index 3c0d4c9b055bcce..672e8b1b6ac9782 100644
--- a/llvm/test/Transforms/InstCombine/phi.ll
+++ b/llvm/test/Transforms/InstCombine/phi.ll
@@ -1316,6 +1316,36 @@ if.end:                                           ; preds = %entry, %if.then
   ret i1  %cmp1
 }
 
+define i1 @phi_knownnonzero_ne_oricmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
+; CHECK-LABEL: @phi_knownnonzero_ne_oricmp(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp slt i32 [[N:%.*]], [[S:%.*]]
+; CHECK-NEXT:    br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    [[A_0:%.*]] = phi i32 [ 1, [[IF_THEN]] ], [ [[N]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[TMP0:%.*]] = or i32 [[A_0]], [[VAL:%.*]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ne i32 [[TMP0]], 0
+; CHECK-NEXT:    ret i1 [[CMP1]]
+;
+entry:
+  %tobool = icmp slt  i32 %n, %s
+  br i1 %tobool, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = load i32, ptr %P
+  %cmp = icmp eq i32 %n, %0
+  %1 = select i1 %cmp, i32 1, i32 2
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.then
+  %a.0 = phi i32 [ %1,  %if.then ], [ %n, %entry ]
+  %2 = or i32 %a.0, %val
+  %cmp1 = icmp ne i32 %2, 0
+  ret i1  %cmp1
+}
+
 define i1 @phi_knownnonzero_eq_multiuse_oricmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
 ; CHECK-LABEL: @phi_knownnonzero_eq_multiuse_oricmp(
 ; CHECK-NEXT:  entry:
@@ -1360,6 +1390,50 @@ cleanup:
   ret i1  %final
 }
 
+define i1 @phi_knownnonzero_ne_multiuse_oricmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
+; CHECK-LABEL: @phi_knownnonzero_ne_multiuse_oricmp(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp slt i32 [[N:%.*]], [[S:%.*]]
+; CHECK-NEXT:    br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    [[A_0:%.*]] = phi i32 [ 1, [[IF_THEN]] ], [ [[N]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[TMP0:%.*]] = or i32 [[A_0]], [[VAL:%.*]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ne i32 [[TMP0]], 0
+; CHECK-NEXT:    br i1 [[CMP1]], label [[NEXT:%.*]], label [[CLEANUP:%.*]]
+; CHECK:       next:
+; CHECK-NEXT:    [[BOOL2:%.*]] = icmp ne i32 [[A_0]], 0
+; CHECK-NEXT:    br label [[CLEANUP]]
+; CHECK:       cleanup:
+; CHECK-NEXT:    [[FINAL:%.*]] = phi i1 [ [[CMP1]], [[IF_END]] ], [ [[BOOL2]], [[NEXT]] ]
+; CHECK-NEXT:    ret i1 [[FINAL]]
+;
+entry:
+  %tobool = icmp slt  i32 %n, %s
+  br i1 %tobool, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = load i32, ptr %P
+  %cmp = icmp eq i32 %n, %0
+  %1 = select i1 %cmp, i32 1, i32 2
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.then
+  %a.0 = phi i32 [ %1,  %if.then ], [ %n, %entry ]
+  %2 = or i32 %a.0, %val
+  %cmp1 = icmp ne i32 %2, 0
+  br i1 %cmp1, label %next, label %cleanup
+
+next:
+  %bool2 = icmp ne i32 %a.0, 0
+  br label %cleanup
+
+cleanup:
+  %final =  phi i1 [ %cmp1,  %if.end ], [ %bool2, %next ]
+  ret i1  %final
+}
+
 define i1 @phi_knownnonzero_eq_multiuse_andicmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
 ; CHECK-LABEL: @phi_knownnonzero_eq_multiuse_andicmp(
 ; CHECK-NEXT:  entry:
@@ -1407,6 +1481,53 @@ cleanup:
   ret i1  %final
 }
 
+define i1 @phi_knownnonzero_ne_multiuse_andicmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
+; CHECK-LABEL: @phi_knownnonzero_ne_multiuse_andicmp(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp slt i32 [[N:%.*]], [[S:%.*]]
+; CHECK-NEXT:    br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr [[P:%.*]], align 4
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[N]]
+; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[CMP]], i32 1, i32 2
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    [[A_0:%.*]] = phi i32 [ [[TMP1]], [[IF_THEN]] ], [ [[N]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[TMP2:%.*]] = and i32 [[A_0]], [[VAL:%.*]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ne i32 [[TMP2]], 0
+; CHECK-NEXT:    br i1 [[CMP1]], label [[NEXT:%.*]], label [[CLEANUP:%.*]]
+; CHECK:       next:
+; CHECK-NEXT:    [[BOOL2:%.*]] = icmp ne i32 [[A_0]], 0
+; CHECK-NEXT:    br label [[CLEANUP]]
+; CHECK:       cleanup:
+; CHECK-NEXT:    [[FINAL:%.*]] = phi i1 [ [[CMP1]], [[IF_END]] ], [ [[BOOL2]], [[NEXT]] ]
+; CHECK-NEXT:    ret i1 [[FINAL]]
+;
+entry:
+  %tobool = icmp slt  i32 %n, %s
+  br i1 %tobool, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %0 = load i32, ptr %P
+  %cmp = icmp eq i32 %n, %0
+  %1 = select i1 %cmp, i32 1, i32 2
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.then
+  %a.0 = phi i32 [ %1,  %if.then ], [ %n, %entry ]
+  %2 = and i32 %a.0, %val
+  %cmp1 = icmp ne i32 %2, 0
+  br i1 %cmp1, label %next, label %cleanup
+
+next:
+  %bool2 = icmp ne i32 %a.0, 0
+  br label %cleanup
+
+cleanup:
+  %final =  phi i1 [ %cmp1,  %if.end ], [ %bool2, %next ]
+  ret i1  %final
+}
+
 ; This would crash trying to delete an instruction (conv)
 ; that still had uses because the user (the phi) was not
 ; updated to remove a use from an unreachable block (g.exit).

>From 0a37a28ab176d54c972104d7bed779e95ef90ba1 Mon Sep 17 00:00:00 2001
From: bipmis <biplob.mishra at arm.com>
Date: Thu, 5 Oct 2023 13:43:05 +0100
Subject: [PATCH 3/4] Update the comments on phi uses as per review

---
 llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index db21c7da22d600d..537063409db0237 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -1452,9 +1452,9 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) {
   // %p = phi [%v, BB] ...
   //      icmp eq, %p, 0
   // FIXME: To be simple, handle only integer type for now.
-  // Extend to 2 use of phi -> icmp and or(icmp). Putting a limit on number of
-  // uses as it turns a O(1) algorithm into a O(n) algorithm. And if we are
-  // doing it for each instruction it turns into O(n^2).
+  // This handles a small number of uses to keep the complexity down, and an
+  // icmp(or(phi)) can equally be replaced with any non-zero constant as the
+  // "or" will only add bits.
   if (PN.hasOneUse() || PN.hasNUses(2)) {
     bool AllUsesOfPhiEndsInCmp = true;
     for (const auto *U : PN.users()) {

>From 8fdd84efacda801d540c83eccffb94eeb7522f11 Mon Sep 17 00:00:00 2001
From: bipmis <biplob.mishra at arm.com>
Date: Thu, 5 Oct 2023 13:59:12 +0100
Subject: [PATCH 4/4] Update to use hasNUsesOrMore

---
 llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index 537063409db0237..0d93e6e250693ee 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -1455,7 +1455,7 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) {
   // This handles a small number of uses to keep the complexity down, and an
   // icmp(or(phi)) can equally be replaced with any non-zero constant as the
   // "or" will only add bits.
-  if (PN.hasOneUse() || PN.hasNUses(2)) {
+  if (!PN.hasNUsesOrMore(3)) {
     bool AllUsesOfPhiEndsInCmp = true;
     for (const auto *U : PN.users()) {
       auto *CmpInst = dyn_cast<ICmpInst>(U);



More information about the llvm-commits mailing list