[llvm] [SSAUpdaterBulk] Add PHI simplification pass. (PR #132004)

Valery Pykhtin via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 21 04:07:41 PDT 2025


https://github.com/vpykhtin updated https://github.com/llvm/llvm-project/pull/132004

>From 85969613f99b3c96d6d7d7f3d5e0b68e69b68600 Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Wed, 19 Mar 2025 11:05:18 +0000
Subject: [PATCH 1/4] [SSAUpdaterBulk] Add PHI simplification pass.

---
 .../llvm/Transforms/Utils/SSAUpdaterBulk.h    | 24 ++++++++++
 llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp  | 47 +++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/llvm/include/llvm/Transforms/Utils/SSAUpdaterBulk.h b/llvm/include/llvm/Transforms/Utils/SSAUpdaterBulk.h
index ad24cb454d5e7..6894e4fa0a357 100644
--- a/llvm/include/llvm/Transforms/Utils/SSAUpdaterBulk.h
+++ b/llvm/include/llvm/Transforms/Utils/SSAUpdaterBulk.h
@@ -79,6 +79,30 @@ class SSAUpdaterBulk {
   /// vector.
   void RewriteAllUses(DominatorTree *DT,
                       SmallVectorImpl<PHINode *> *InsertedPHIs = nullptr);
+
+  /// Rewrite all uses and simplify the inserted PHI nodes.
+  /// Use this method to preserve behavior when replacing SSAUpdater.
+  void RewriteAndSimplifyAllUses(DominatorTree *DT) {
+    SmallVector<PHINode *, 8> NewPHIs;
+    RewriteAllUses(DT, &NewPHIs);
+    simplifyPass(NewPHIs);
+  }
+
+  /// Same as previous, but return inserted PHI nodes in InsertedPHIs.
+  void RewriteAndSimplifyAllUses(DominatorTree *DT,
+                                 SmallVectorImpl<PHINode *> &InsertedPHIs) {
+    RewriteAllUses(DT, &InsertedPHIs);
+    simplifyPass(InsertedPHIs);
+    // Remove nullptrs from the worklist
+    InsertedPHIs.erase(
+        std::remove(InsertedPHIs.begin(), InsertedPHIs.end(), nullptr),
+        InsertedPHIs.end());
+  }
+
+private:
+  /// Perform a single pass of simplification over the worklist of PHIs.
+  /// Pointers to replaced phis are nulled out.
+  static bool simplifyPass(SmallVectorImpl<PHINode *> &Worklist);
 };
 
 } // end namespace llvm
diff --git a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
index cad7ff64c01fb..dac964b092ec5 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Utils/SSAUpdaterBulk.h"
+#include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/IteratedDominanceFrontier.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Dominators.h"
@@ -182,3 +183,49 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
     }
   }
 }
+
+static bool isEquivalentPHI(PHINode *PHI1, PHINode *PHI2) {
+  if (PHI1->getNumIncomingValues() != PHI2->getNumIncomingValues())
+    return false;
+
+  unsigned I = 0, NumValues = PHI1->getNumIncomingValues();
+  for (; I != NumValues; ++I) {
+    if (PHI1->getIncomingBlock(I) != PHI2->getIncomingBlock(I))
+      break;
+    if (PHI1->getIncomingValue(I) != PHI2->getIncomingValue(I))
+      return false;
+  }
+  // TODO: add procesing if phis have different order of incoming values.
+  return I == NumValues;
+}
+
+bool SSAUpdaterBulk::simplifyPass(SmallVectorImpl<PHINode *> &Worklist) {
+  if (Worklist.empty())
+    return false;
+
+  auto findEquivalentPHI = [](PHINode *PHI) -> Value * {
+    BasicBlock *BB = PHI->getParent();
+    for (auto &OtherPHI : BB->phis()) {
+      if (PHI != &OtherPHI && isEquivalentPHI(PHI, &OtherPHI)) {
+        return &OtherPHI;
+      }
+    }
+    return nullptr;
+  };
+
+  const DataLayout &DL = Worklist.front()->getParent()->getDataLayout();
+  bool Change = false;
+  for (PHINode *&PHI : Worklist) {
+    Value *Replacement = simplifyInstruction(PHI, DL);
+    if (!Replacement)
+      Replacement = findEquivalentPHI(PHI);
+
+    if (Replacement) {
+      PHI->replaceAllUsesWith(Replacement);
+      PHI->eraseFromParent();
+      PHI = nullptr; // Mark as removed
+      Change = true;
+    }
+  }
+  return Change;
+}

>From 10e99d6f5b9529cd8c2f2861441047b84dbb9518 Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Wed, 19 Mar 2025 17:36:59 +0000
Subject: [PATCH 2/4] add test.

---
 .../Transforms/Utils/SSAUpdaterBulkTest.cpp   | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp b/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
index cfd7fbb445840..9d490dc4c6694 100644
--- a/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
+++ b/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
@@ -311,3 +311,69 @@ TEST(SSAUpdaterBulk, TwoBBLoop) {
   EXPECT_EQ(Phi->getIncomingValueForBlock(Entry), ConstantInt::get(I32Ty, 0));
   EXPECT_EQ(Phi->getIncomingValueForBlock(Loop), I);
 }
+
+TEST(SSAUpdaterBulk, SimplifyPHIs) {
+  const char *IR = R"(
+      define void @main(i32 %val, i1 %cond) {
+      entry:
+          br i1 %cond, label %left, label %right
+      left:
+          %add = add i32 %val, 1
+          br label %exit
+      right:
+          %sub = sub i32 %val, 1
+          br label %exit
+      exit:
+          %phi = phi i32 [ %sub, %right ], [ %add, %left ]
+          %cmp = icmp slt i32 0, 42
+          ret void
+      }
+  )";
+
+  llvm::LLVMContext Context;
+  llvm::SMDiagnostic Err;
+  std::unique_ptr<llvm::Module> M = llvm::parseAssemblyString(IR, Err, Context);
+  ASSERT_NE(M, nullptr) << "Failed to parse IR: " << Err.getMessage();
+
+  Function *F = M->getFunction("main");
+  auto *Entry = &F->getEntryBlock();
+  auto *Left = Entry->getTerminator()->getSuccessor(0);
+  auto *Right = Entry->getTerminator()->getSuccessor(1);
+  auto *Exit = Left->getSingleSuccessor();
+  auto *Val = &*F->arg_begin();
+  auto *Phi = &Exit->front();
+  auto *Cmp = &*std::next(Exit->begin());
+  auto *Add = &Left->front();
+  auto *Sub = &Right->front();
+
+  SSAUpdaterBulk Updater;
+  Type *I32Ty = Type::getInt32Ty(Context);
+
+  // Use %val directly instead of creating a phi.
+  unsigned ValVar = Updater.AddVariable("Val", I32Ty);
+  Updater.AddAvailableValue(ValVar, Left, Val);
+  Updater.AddAvailableValue(ValVar, Right, Val);
+  Updater.AddUse(ValVar, &Cmp->getOperandUse(0));
+
+  // Use existing %phi for %add and %sub values.
+  unsigned AddSubVar = Updater.AddVariable("AddSub", I32Ty);
+  Updater.AddAvailableValue(AddSubVar, Left, Add);
+  Updater.AddAvailableValue(AddSubVar, Right, Sub);
+  Updater.AddUse(AddSubVar, &Cmp->getOperandUse(1));
+
+  SmallVector<PHINode *, 2> Inserted;
+  DominatorTree DT(*F);
+  Updater.RewriteAndSimplifyAllUses(&DT, Inserted);
+
+#if 0 // Enable for debugging.
+  Exit->dump();
+  //  Output:
+  //  exit:                                             ; preds = %right, %left
+  //    %phi = phi i32 [ %sub, %right ], [ %add, %left ]
+  //    %cmp = icmp slt i32 %val, %phi
+  //    ret void
+#endif
+  EXPECT_EQ(Inserted.size(), 0u);
+  EXPECT_EQ(Val, Cmp->getOperand(0));
+  EXPECT_EQ(Phi, Cmp->getOperand(1));
+}

>From 241b30001722f984cd6a3e8aefd0233fc0afbc83 Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Fri, 21 Mar 2025 10:09:08 +0000
Subject: [PATCH 3/4] Use isIdenticalToWhenDefined

---
 llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
index b24ef4dae43fc..7edfa3940895e 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
@@ -225,21 +225,6 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
   }
 }
 
-static bool isEquivalentPHI(PHINode *PHI1, PHINode *PHI2) {
-  if (PHI1->getNumIncomingValues() != PHI2->getNumIncomingValues())
-    return false;
-
-  unsigned I = 0, NumValues = PHI1->getNumIncomingValues();
-  for (; I != NumValues; ++I) {
-    if (PHI1->getIncomingBlock(I) != PHI2->getIncomingBlock(I))
-      break;
-    if (PHI1->getIncomingValue(I) != PHI2->getIncomingValue(I))
-      return false;
-  }
-  // TODO: add procesing if phis have different order of incoming values.
-  return I == NumValues;
-}
-
 bool SSAUpdaterBulk::simplifyPass(SmallVectorImpl<PHINode *> &Worklist) {
   if (Worklist.empty())
     return false;
@@ -247,9 +232,8 @@ bool SSAUpdaterBulk::simplifyPass(SmallVectorImpl<PHINode *> &Worklist) {
   auto findEquivalentPHI = [](PHINode *PHI) -> Value * {
     BasicBlock *BB = PHI->getParent();
     for (auto &OtherPHI : BB->phis()) {
-      if (PHI != &OtherPHI && isEquivalentPHI(PHI, &OtherPHI)) {
+      if (PHI != &OtherPHI && PHI->isIdenticalToWhenDefined(&OtherPHI)) 
         return &OtherPHI;
-      }
     }
     return nullptr;
   };

>From fba85b40dfadec9b05e4648bc72d99609492dcde Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Fri, 21 Mar 2025 11:07:21 +0000
Subject: [PATCH 4/4] formatting

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

diff --git a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
index 7edfa3940895e..4e59555068288 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
@@ -232,7 +232,7 @@ bool SSAUpdaterBulk::simplifyPass(SmallVectorImpl<PHINode *> &Worklist) {
   auto findEquivalentPHI = [](PHINode *PHI) -> Value * {
     BasicBlock *BB = PHI->getParent();
     for (auto &OtherPHI : BB->phis()) {
-      if (PHI != &OtherPHI && PHI->isIdenticalToWhenDefined(&OtherPHI)) 
+      if (PHI != &OtherPHI && PHI->isIdenticalToWhenDefined(&OtherPHI))
         return &OtherPHI;
     }
     return nullptr;



More information about the llvm-commits mailing list