[llvm] [SandboxIR] Implement missing PHINode functions (PR #101734)

via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 5 11:11:05 PDT 2024


https://github.com/Sterling-Augustine updated https://github.com/llvm/llvm-project/pull/101734

>From 8ea3fbf24bd71583649cc98e8e088640a3fde8ff Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Fri, 2 Aug 2024 18:48:14 +0000
Subject: [PATCH 1/4] Implement missing PHINode functions

replaceIncomingBlockWith and removeIncomingValueIf are both
straightforward and done.

I'll defer copyIncomingBlocks until a couple of other changes that
also handle blocks go in.
---
 llvm/include/llvm/SandboxIR/SandboxIR.h    |  8 ++++----
 llvm/lib/SandboxIR/SandboxIR.cpp           | 16 ++++++++++++++++
 llvm/unittests/SandboxIR/SandboxIRTest.cpp | 13 ++++++++++++-
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/SandboxIR/SandboxIR.h b/llvm/include/llvm/SandboxIR/SandboxIR.h
index 0bd6cdac36f9a..d47bb4e87c219 100644
--- a/llvm/include/llvm/SandboxIR/SandboxIR.h
+++ b/llvm/include/llvm/SandboxIR/SandboxIR.h
@@ -1585,12 +1585,12 @@ class PHINode final : public Instruction {
     return cast<llvm::PHINode>(Val)->hasConstantOrUndefValue();
   }
   bool isComplete() const { return cast<llvm::PHINode>(Val)->isComplete(); }
-  // TODO: Implement the below functions:
-  // void replaceIncomingBlockWith (const BasicBlock *Old, BasicBlock *New);
+  void replaceIncomingBlockWith (const BasicBlock *Old, BasicBlock *New);
+  void removeIncomingValueIf(function_ref< bool(unsigned)> Predicate,
+                             bool DeletePHIIfEmpty=true);
+  // TODO: Implement 
   // void copyIncomingBlocks(iterator_range<const_block_iterator> BBRange,
   //                         uint32_t ToIdx = 0)
-  // void removeIncomingValueIf(function_ref< bool(unsigned)> Predicate,
-  //                            bool DeletePHIIfEmpty=true)
 #ifndef NDEBUG
   void verify() const final {
     assert(isa<llvm::PHINode>(Val) && "Expected PHINode!");
diff --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp
index a67cb2a5bb25a..f34eeee2399ac 100644
--- a/llvm/lib/SandboxIR/SandboxIR.cpp
+++ b/llvm/lib/SandboxIR/SandboxIR.cpp
@@ -1150,6 +1150,22 @@ Value *PHINode::hasConstantValue() const {
   llvm::Value *LLVMV = cast<llvm::PHINode>(Val)->hasConstantValue();
   return LLVMV != nullptr ? Ctx.getValue(LLVMV) : nullptr;
 }
+void PHINode::replaceIncomingBlockWith (const BasicBlock *Old, BasicBlock *New) {
+  assert(New && Old && "Sandbox IR PHI node got a null basic block!");
+  for (unsigned Op = 0,
+            NumOps = cast<llvm::PHINode>(Val)->getNumOperands(); Op != NumOps; ++Op)
+    if (getIncomingBlock(Op) == Old)
+      setIncomingBlock(Op, New);
+}
+void PHINode::removeIncomingValueIf(function_ref< bool(unsigned)> Predicate,
+                                    bool DeletePHIIfEmpty) {
+  // Avoid duplicate tracking by going through this->removeIncomingValue here at
+  // the expense of some performance. Copy PHI::removeIncomingValueIf more
+  // directly if performance becomes an issue.
+  for (unsigned Idx = 0; Idx < getNumIncomingValues(); ++Idx)
+    if (Predicate(Idx))
+      removeIncomingValue(Idx);
+}
 
 static llvm::Instruction::CastOps getLLVMCastOp(Instruction::Opcode Opc) {
   switch (Opc) {
diff --git a/llvm/unittests/SandboxIR/SandboxIRTest.cpp b/llvm/unittests/SandboxIR/SandboxIRTest.cpp
index 5f858b47926ec..5d22f3b003247 100644
--- a/llvm/unittests/SandboxIR/SandboxIRTest.cpp
+++ b/llvm/unittests/SandboxIR/SandboxIRTest.cpp
@@ -1974,7 +1974,18 @@ define void @foo(i32 %arg) {
   EXPECT_EQ(PHI->hasConstantOrUndefValue(), LLVMPHI->hasConstantOrUndefValue());
   // Check isComplete().
   EXPECT_EQ(PHI->isComplete(), LLVMPHI->isComplete());
-
+  // Check replaceIncomingBlockWith
+  OrigBB = PHI->getIncomingBlock(0);
+  EXPECT_EQ(OrigBB, BB1);
+  EXPECT_NE(OrigBB, BB2);
+  PHI->replaceIncomingBlockWith(BB1, BB2);
+  EXPECT_EQ(PHI->getIncomingBlock(0), BB2);
+  // Check replaceIncomingValueIf
+  EXPECT_EQ(PHI->getNumIncomingValues(), 2u);
+  PHI->removeIncomingValueIf([&](unsigned Idx) {
+    return PHI->getIncomingBlock(Idx) == BB2;
+  });
+  EXPECT_EQ(PHI->getNumIncomingValues(), 1u);
   // Check create().
   auto *NewPHI = cast<sandboxir::PHINode>(
       sandboxir::PHINode::create(PHI->getType(), 0, Br, Ctx, "NewPHI"));

>From 3dc7660373a325d967430da18d7742d888487360 Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Fri, 2 Aug 2024 21:30:17 +0000
Subject: [PATCH 2/4] [SandboxIR] Checkpoint changes.

---
 llvm/lib/SandboxIR/SandboxIR.cpp           | 10 +++++-----
 llvm/unittests/SandboxIR/SandboxIRTest.cpp |  9 +++++----
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp
index f34eeee2399ac..c171c449afc01 100644
--- a/llvm/lib/SandboxIR/SandboxIR.cpp
+++ b/llvm/lib/SandboxIR/SandboxIR.cpp
@@ -1152,10 +1152,10 @@ Value *PHINode::hasConstantValue() const {
 }
 void PHINode::replaceIncomingBlockWith (const BasicBlock *Old, BasicBlock *New) {
   assert(New && Old && "Sandbox IR PHI node got a null basic block!");
-  for (unsigned Op = 0,
-            NumOps = cast<llvm::PHINode>(Val)->getNumOperands(); Op != NumOps; ++Op)
-    if (getIncomingBlock(Op) == Old)
-      setIncomingBlock(Op, New);
+  for (unsigned Idx = 0,
+            NumOps = cast<llvm::PHINode>(Val)->getNumOperands(); Idx != NumOps; ++Idx)
+    if (getIncomingBlock(Idx) == Old)
+      setIncomingBlock(Idx, New);
 }
 void PHINode::removeIncomingValueIf(function_ref< bool(unsigned)> Predicate,
                                     bool DeletePHIIfEmpty) {
@@ -1164,7 +1164,7 @@ void PHINode::removeIncomingValueIf(function_ref< bool(unsigned)> Predicate,
   // directly if performance becomes an issue.
   for (unsigned Idx = 0; Idx < getNumIncomingValues(); ++Idx)
     if (Predicate(Idx))
-      removeIncomingValue(Idx);
+      removeIncomingValue(Idx, DeletePHIIfEmpty);
 }
 
 static llvm::Instruction::CastOps getLLVMCastOp(Instruction::Opcode Opc) {
diff --git a/llvm/unittests/SandboxIR/SandboxIRTest.cpp b/llvm/unittests/SandboxIR/SandboxIRTest.cpp
index 5d22f3b003247..694e0c6a97ef8 100644
--- a/llvm/unittests/SandboxIR/SandboxIRTest.cpp
+++ b/llvm/unittests/SandboxIR/SandboxIRTest.cpp
@@ -1976,16 +1976,17 @@ define void @foo(i32 %arg) {
   EXPECT_EQ(PHI->isComplete(), LLVMPHI->isComplete());
   // Check replaceIncomingBlockWith
   OrigBB = PHI->getIncomingBlock(0);
-  EXPECT_EQ(OrigBB, BB1);
-  EXPECT_NE(OrigBB, BB2);
-  PHI->replaceIncomingBlockWith(BB1, BB2);
-  EXPECT_EQ(PHI->getIncomingBlock(0), BB2);
+  auto *NewBB = BB2;
+  EXPECT_NE(NewBB, OrigBB);
+  PHI->replaceIncomingBlockWith(OrigBB, NewBB);
+  EXPECT_EQ(PHI->getIncomingBlock(0), NewBB);
   // Check replaceIncomingValueIf
   EXPECT_EQ(PHI->getNumIncomingValues(), 2u);
   PHI->removeIncomingValueIf([&](unsigned Idx) {
     return PHI->getIncomingBlock(Idx) == BB2;
   });
   EXPECT_EQ(PHI->getNumIncomingValues(), 1u);
+  EXPECT_EQ(PHI->getIncomingBlock(0), BB1);
   // Check create().
   auto *NewPHI = cast<sandboxir::PHINode>(
       sandboxir::PHINode::create(PHI->getType(), 0, Br, Ctx, "NewPHI"));

>From b2da0ee715a040bc5eabf2787b9f95ee0449b9c3 Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Mon, 5 Aug 2024 18:00:28 +0000
Subject: [PATCH 3/4] Address comments.

Especially improve removal test.
---
 llvm/include/llvm/SandboxIR/SandboxIR.h    |  3 +--
 llvm/lib/SandboxIR/SandboxIR.cpp           | 15 ++++++++++-----
 llvm/unittests/SandboxIR/SandboxIRTest.cpp |  9 +++++----
 llvm/unittests/SandboxIR/TrackerTest.cpp   |  9 +++++++++
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/llvm/include/llvm/SandboxIR/SandboxIR.h b/llvm/include/llvm/SandboxIR/SandboxIR.h
index d47bb4e87c219..0d6b3cf4b9212 100644
--- a/llvm/include/llvm/SandboxIR/SandboxIR.h
+++ b/llvm/include/llvm/SandboxIR/SandboxIR.h
@@ -1586,8 +1586,7 @@ class PHINode final : public Instruction {
   }
   bool isComplete() const { return cast<llvm::PHINode>(Val)->isComplete(); }
   void replaceIncomingBlockWith (const BasicBlock *Old, BasicBlock *New);
-  void removeIncomingValueIf(function_ref< bool(unsigned)> Predicate,
-                             bool DeletePHIIfEmpty=true);
+  void removeIncomingValueIf(function_ref< bool(unsigned)> Predicate);
   // TODO: Implement 
   // void copyIncomingBlocks(iterator_range<const_block_iterator> BBRange,
   //                         uint32_t ToIdx = 0)
diff --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp
index c171c449afc01..4a2137efb3979 100644
--- a/llvm/lib/SandboxIR/SandboxIR.cpp
+++ b/llvm/lib/SandboxIR/SandboxIR.cpp
@@ -1157,14 +1157,19 @@ void PHINode::replaceIncomingBlockWith (const BasicBlock *Old, BasicBlock *New)
     if (getIncomingBlock(Idx) == Old)
       setIncomingBlock(Idx, New);
 }
-void PHINode::removeIncomingValueIf(function_ref< bool(unsigned)> Predicate,
-                                    bool DeletePHIIfEmpty) {
+void PHINode::removeIncomingValueIf(function_ref< bool(unsigned)> Predicate) {
   // Avoid duplicate tracking by going through this->removeIncomingValue here at
   // the expense of some performance. Copy PHI::removeIncomingValueIf more
   // directly if performance becomes an issue.
-  for (unsigned Idx = 0; Idx < getNumIncomingValues(); ++Idx)
-    if (Predicate(Idx))
-      removeIncomingValue(Idx, DeletePHIIfEmpty);
+  unsigned Idx = 0;
+  unsigned LastIdx = getNumIncomingValues();
+  while (Idx < LastIdx) {
+    if (Predicate(Idx)) {
+      removeIncomingValue(Idx);
+      --LastIdx;
+    } else
+      ++Idx;
+  }
 }
 
 static llvm::Instruction::CastOps getLLVMCastOp(Instruction::Opcode Opc) {
diff --git a/llvm/unittests/SandboxIR/SandboxIRTest.cpp b/llvm/unittests/SandboxIR/SandboxIRTest.cpp
index 694e0c6a97ef8..51edf604280b0 100644
--- a/llvm/unittests/SandboxIR/SandboxIRTest.cpp
+++ b/llvm/unittests/SandboxIR/SandboxIRTest.cpp
@@ -1876,10 +1876,11 @@ define void @foo(i32 %arg) {
   br label %bb2
 
 bb2:
-  %phi = phi i32 [ %arg, %bb1 ], [ 0, %bb2 ]
+  %phi = phi i32 [ %arg, %bb1 ], [ 0, %bb2 ], [ 1, %bb3]
   br label %bb2
 
 bb3:
+  br label %bb2
   ret void
 }
 )IR");
@@ -1981,12 +1982,12 @@ define void @foo(i32 %arg) {
   PHI->replaceIncomingBlockWith(OrigBB, NewBB);
   EXPECT_EQ(PHI->getIncomingBlock(0), NewBB);
   // Check replaceIncomingValueIf
-  EXPECT_EQ(PHI->getNumIncomingValues(), 2u);
+  EXPECT_EQ(PHI->getNumIncomingValues(), 3u);
   PHI->removeIncomingValueIf([&](unsigned Idx) {
-    return PHI->getIncomingBlock(Idx) == BB2;
+    return PHI->getIncomingBlock(Idx) == NewBB;
   });
   EXPECT_EQ(PHI->getNumIncomingValues(), 1u);
-  EXPECT_EQ(PHI->getIncomingBlock(0), BB1);
+  EXPECT_EQ(PHI->getIncomingBlock(0), BB3);
   // Check create().
   auto *NewPHI = cast<sandboxir::PHINode>(
       sandboxir::PHINode::create(PHI->getType(), 0, Br, Ctx, "NewPHI"));
diff --git a/llvm/unittests/SandboxIR/TrackerTest.cpp b/llvm/unittests/SandboxIR/TrackerTest.cpp
index d016c7793a52c..cabc1a575e2bc 100644
--- a/llvm/unittests/SandboxIR/TrackerTest.cpp
+++ b/llvm/unittests/SandboxIR/TrackerTest.cpp
@@ -680,6 +680,15 @@ define void @foo(i8 %arg0, i8 %arg1, i8 %arg2) {
   EXPECT_EQ(PHI->getIncomingBlock(1), BB1);
   EXPECT_EQ(PHI->getIncomingValue(1), Arg1);
 
+  // Check removeIncomingValueIf(FromBB1).
+  Ctx.save();
+  PHI->removeIncomingValueIf([&](unsigned Idx) {
+    return PHI->getIncomingBlock(Idx) == BB1;
+  });
+  EXPECT_EQ(PHI->getNumIncomingValues(), 1u);
+  Ctx.revert();
+  EXPECT_EQ(PHI->getNumIncomingValues(), 2u);
+
   // Check removeIncomingValue() remove all.
   Ctx.save();
   PHI->removeIncomingValue(0u);

>From 8591fe21a0c8f20ea7efb905941c2d7646106cd4 Mon Sep 17 00:00:00 2001
From: Sterling Augustine <saugustine at google.com>
Date: Mon, 5 Aug 2024 18:10:00 +0000
Subject: [PATCH 4/4] Fix formatting

---
 llvm/include/llvm/SandboxIR/SandboxIR.h | 2 +-
 llvm/lib/SandboxIR/SandboxIR.cpp        | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/llvm/include/llvm/SandboxIR/SandboxIR.h b/llvm/include/llvm/SandboxIR/SandboxIR.h
index 0d6b3cf4b9212..4a5d4019ed0bd 100644
--- a/llvm/include/llvm/SandboxIR/SandboxIR.h
+++ b/llvm/include/llvm/SandboxIR/SandboxIR.h
@@ -1585,7 +1585,7 @@ class PHINode final : public Instruction {
     return cast<llvm::PHINode>(Val)->hasConstantOrUndefValue();
   }
   bool isComplete() const { return cast<llvm::PHINode>(Val)->isComplete(); }
-  void replaceIncomingBlockWith (const BasicBlock *Old, BasicBlock *New);
+  void replaceIncomingBlockWith(const BasicBlock *Old, BasicBlock *New);
   void removeIncomingValueIf(function_ref< bool(unsigned)> Predicate);
   // TODO: Implement 
   // void copyIncomingBlocks(iterator_range<const_block_iterator> BBRange,
diff --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp
index 4a2137efb3979..b28ce0ce775b5 100644
--- a/llvm/lib/SandboxIR/SandboxIR.cpp
+++ b/llvm/lib/SandboxIR/SandboxIR.cpp
@@ -1150,10 +1150,10 @@ Value *PHINode::hasConstantValue() const {
   llvm::Value *LLVMV = cast<llvm::PHINode>(Val)->hasConstantValue();
   return LLVMV != nullptr ? Ctx.getValue(LLVMV) : nullptr;
 }
-void PHINode::replaceIncomingBlockWith (const BasicBlock *Old, BasicBlock *New) {
+void PHINode::replaceIncomingBlockWith(const BasicBlock *Old, BasicBlock *New) {
   assert(New && Old && "Sandbox IR PHI node got a null basic block!");
-  for (unsigned Idx = 0,
-            NumOps = cast<llvm::PHINode>(Val)->getNumOperands(); Idx != NumOps; ++Idx)
+  for (unsigned Idx = 0, NumOps = cast<llvm::PHINode>(Val)->getNumOperands();
+       Idx != NumOps; ++Idx)
     if (getIncomingBlock(Idx) == Old)
       setIncomingBlock(Idx, New);
 }



More information about the llvm-commits mailing list