[llvm] [InstCombine] Remove dead phi web (PR #108876)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 17 17:06:08 PDT 2024
https://github.com/Chengjunp updated https://github.com/llvm/llvm-project/pull/108876
>From c520b90745c14d27ef5fcb163afb65f09ff0d70f Mon Sep 17 00:00:00 2001
From: chengjunp <chengjunp at nvidia.com>
Date: Mon, 16 Sep 2024 20:22:44 +0000
Subject: [PATCH 1/3] Remove dead phi web in InstCombinePHI
---
.../Transforms/InstCombine/InstCombinePHI.cpp | 57 +++++++++----------
llvm/test/Transforms/InstCombine/phi.ll | 51 +++++++++++++++++
2 files changed, 79 insertions(+), 29 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index bcff9a72b65724..697e7b35033cd9 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -976,24 +976,26 @@ Instruction *InstCombinerImpl::foldPHIArgOpIntoPHI(PHINode &PN) {
return NewCI;
}
-/// Return true if this PHI node is only used by a PHI node cycle that is dead.
-static bool isDeadPHICycle(PHINode *PN,
- SmallPtrSetImpl<PHINode *> &PotentiallyDeadPHIs) {
- if (PN->use_empty()) return true;
- if (!PN->hasOneUse()) return false;
-
- // Remember this node, and if we find the cycle, return.
- if (!PotentiallyDeadPHIs.insert(PN).second)
- return true;
-
- // Don't scan crazily complex things.
- if (PotentiallyDeadPHIs.size() == 16)
- return false;
-
- if (PHINode *PU = dyn_cast<PHINode>(PN->user_back()))
- return isDeadPHICycle(PU, PotentiallyDeadPHIs);
-
- return false;
+/// Return true if this PHI node is only used by a PHI node web that is dead.
+static bool isDeadPHIWeb(PHINode *PN) {
+ SmallVector<PHINode *, 16> Stack;
+ SmallPtrSet<PHINode *, 16> Visited;
+ Stack.push_back(PN);
+ while (!Stack.empty()) {
+ PHINode *Phi = Stack.pop_back_val();
+ if (!Visited.insert(Phi).second)
+ continue;
+ // Early stop if the set of PHIs is large
+ if (Visited.size() == 16)
+ return false;
+ for (User *Use : Phi->users()) {
+ PHINode *PhiUse = dyn_cast<PHINode>(Use);
+ if (!PhiUse)
+ return false;
+ Stack.push_back(PhiUse);
+ }
+ }
+ return true;
}
/// Return true if this phi node is always equal to NonPhiInVal.
@@ -1474,27 +1476,24 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) {
}
}
- // If this is a trivial cycle in the PHI node graph, remove it. Basically, if
- // this PHI only has a single use (a PHI), and if that PHI only has one use (a
- // PHI)... break the cycle.
+ // If the phi is within a phi web, which is formed by the def-use chain
+ // of phis and all the phis in the web are only used in the other phis. In
+ // this case, these phis are dead. We can break the web by removing PN.
+ if (isDeadPHIWeb(&PN))
+ return replaceInstUsesWith(PN, PoisonValue::get(PN.getType()));
+
+ // Optimization when the phi only has one use
if (PN.hasOneUse()) {
if (foldIntegerTypedPHI(PN))
return nullptr;
- Instruction *PHIUser = cast<Instruction>(PN.user_back());
- if (PHINode *PU = dyn_cast<PHINode>(PHIUser)) {
- SmallPtrSet<PHINode*, 16> PotentiallyDeadPHIs;
- PotentiallyDeadPHIs.insert(&PN);
- if (isDeadPHICycle(PU, PotentiallyDeadPHIs))
- return replaceInstUsesWith(PN, PoisonValue::get(PN.getType()));
- }
-
// If this phi has a single use, and if that use just computes a value for
// the next iteration of a loop, delete the phi. This occurs with unused
// induction variables, e.g. "for (int j = 0; ; ++j);". Detecting this
// common case here is good because the only other things that catch this
// are induction variable analysis (sometimes) and ADCE, which is only run
// late.
+ Instruction *PHIUser = cast<Instruction>(PN.user_back());
if (PHIUser->hasOneUse() &&
(isa<BinaryOperator>(PHIUser) || isa<UnaryOperator>(PHIUser) ||
isa<GetElementPtrInst>(PHIUser)) &&
diff --git a/llvm/test/Transforms/InstCombine/phi.ll b/llvm/test/Transforms/InstCombine/phi.ll
index 3b1fa3a97d9cd7..b33ad9a7d339f2 100644
--- a/llvm/test/Transforms/InstCombine/phi.ll
+++ b/llvm/test/Transforms/InstCombine/phi.ll
@@ -2742,3 +2742,54 @@ loop.latch:
call void @use(i32 %and)
br label %loop
}
+
+define void @test_dead_phi_web(i64 %index, i1 %cond) {
+; CHECK-LABEL: @test_dead_phi_web(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[BB0:%.*]]
+; CHECK: BB0:
+; CHECK-NEXT: switch i64 [[INDEX:%.*]], label [[BB4:%.*]] [
+; CHECK-NEXT: i64 0, label [[BB1:%.*]]
+; CHECK-NEXT: i64 1, label [[BB2:%.*]]
+; CHECK-NEXT: i64 2, label [[BB3:%.*]]
+; CHECK-NEXT: ]
+; CHECK: BB1:
+; CHECK-NEXT: br i1 [[COND:%.*]], label [[BB2]], label [[BB4]]
+; CHECK: BB2:
+; CHECK-NEXT: br i1 [[COND]], label [[BB3]], label [[BB4]]
+; CHECK: BB3:
+; CHECK-NEXT: br label [[BB4]]
+; CHECK: BB4:
+; CHECK-NEXT: br i1 [[COND]], label [[BB0]], label [[BB5:%.*]]
+; CHECK: BB5:
+; CHECK-NEXT: ret void
+;
+entry:
+ br label %BB0
+
+BB0: ; preds = %BB4, %entry
+ %a = phi float [ 0.0, %entry ], [ %x, %BB4 ]
+ switch i64 %index, label %BB4 [
+ i64 0, label %BB1
+ i64 1, label %BB2
+ i64 2, label %BB3
+ ]
+
+BB1: ; preds = %BB0
+ br i1 %cond, label %BB2, label %BB4
+
+BB2: ; preds = %BB1, %BB0
+ %b = phi float [ 2.0, %BB0 ], [ %a, %BB1 ]
+ br i1 %cond, label %BB3, label %BB4
+
+BB3: ; preds = %BB2, %BB0
+ %c = phi float [ 3.0, %BB0 ], [ %b, %BB2 ]
+ br label %BB4
+
+BB4: ; preds = %BB3, %BB2, %BB1, %BB0
+ %x = phi float [ %a, %BB0 ], [ %a, %BB1 ], [ %b, %BB2 ], [ %c, %BB3 ]
+ br i1 %cond, label %BB0, label %BB5
+
+BB5: ; preds = %BB4
+ ret void
+}
>From 2ab26119d8e6c6fde3f199da72d6bbc5126171ae Mon Sep 17 00:00:00 2001
From: chengjunp <chengjunp at nvidia.com>
Date: Mon, 16 Sep 2024 20:38:53 +0000
Subject: [PATCH 2/3] Simplify code
---
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 697e7b35033cd9..094c74a207eb1d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -989,10 +989,10 @@ static bool isDeadPHIWeb(PHINode *PN) {
if (Visited.size() == 16)
return false;
for (User *Use : Phi->users()) {
- PHINode *PhiUse = dyn_cast<PHINode>(Use);
- if (!PhiUse)
+ if (PHINode *PhiUse = dyn_cast<PHINode>(Use))
+ Stack.push_back(PhiUse);
+ else
return false;
- Stack.push_back(PhiUse);
}
}
return true;
>From 99157f32fc815212628e75f28071edb804703c50 Mon Sep 17 00:00:00 2001
From: chengjunp <chengjunp at nvidia.com>
Date: Wed, 18 Sep 2024 00:08:27 +0000
Subject: [PATCH 3/3] Remove all dead phis in one visitPHINode
---
.../InstCombine/InstCombineInternal.h | 5 ++
.../Transforms/InstCombine/InstCombinePHI.cpp | 56 ++++++++++---------
2 files changed, 35 insertions(+), 26 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index a051a568bfd62e..5e567c738f7757 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -634,6 +634,11 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
Instruction *foldPHIArgZextsIntoPHI(PHINode &PN);
Instruction *foldPHIArgIntToPtrToPHI(PHINode &PN);
+ // If the phi is within a phi web, which is formed by the def-use chain
+ // of phis and all the phis in the web are only used in the other phis.
+ // In this case, these phis are dead and we will remove all of them.
+ bool foldDeadPhiWeb(PHINode &PN);
+
/// If an integer typed PHI has only one use which is an IntToPtr operation,
/// replace the PHI with an existing pointer typed PHI if it exists. Otherwise
/// insert a new pointer typed PHI and replace the original one.
diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index 094c74a207eb1d..8ed3be6bc4a3ce 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -53,6 +53,34 @@ void InstCombinerImpl::PHIArgMergedDebugLoc(Instruction *Inst, PHINode &PN) {
}
}
+// If the phi is within a phi web, which is formed by the def-use chain
+// of phis and all the phis in the web are only used in the other phis.
+// In this case, these phis are dead and we will remove all of them.
+bool InstCombinerImpl::foldDeadPhiWeb(PHINode &PN){
+ SmallVector<PHINode *, 16> Stack;
+ SmallPtrSet<PHINode *, 16> Visited;
+ Stack.push_back(&PN);
+ while (!Stack.empty()) {
+ PHINode *Phi = Stack.pop_back_val();
+ if (!Visited.insert(Phi).second)
+ continue;
+ // Early stop if the set of PHIs is large
+ if (Visited.size() == 16)
+ return false;
+ for (User *Use : Phi->users()) {
+ if (PHINode *PhiUse = dyn_cast<PHINode>(Use))
+ Stack.push_back(PhiUse);
+ else
+ return false;
+ }
+ }
+ for (PHINode *Phi : Visited)
+ replaceInstUsesWith(*Phi, PoisonValue::get(Phi->getType()));
+ for (PHINode *Phi : Visited)
+ eraseInstFromFunction(*Phi);
+ return true;
+}
+
// Replace Integer typed PHI PN if the PHI's value is used as a pointer value.
// If there is an existing pointer typed PHI that produces the same value as PN,
// replace PN and the IntToPtr operation with it. Otherwise, synthesize a new
@@ -976,27 +1004,6 @@ Instruction *InstCombinerImpl::foldPHIArgOpIntoPHI(PHINode &PN) {
return NewCI;
}
-/// Return true if this PHI node is only used by a PHI node web that is dead.
-static bool isDeadPHIWeb(PHINode *PN) {
- SmallVector<PHINode *, 16> Stack;
- SmallPtrSet<PHINode *, 16> Visited;
- Stack.push_back(PN);
- while (!Stack.empty()) {
- PHINode *Phi = Stack.pop_back_val();
- if (!Visited.insert(Phi).second)
- continue;
- // Early stop if the set of PHIs is large
- if (Visited.size() == 16)
- return false;
- for (User *Use : Phi->users()) {
- if (PHINode *PhiUse = dyn_cast<PHINode>(Use))
- Stack.push_back(PhiUse);
- else
- return false;
- }
- }
- return true;
-}
/// Return true if this phi node is always equal to NonPhiInVal.
/// This happens with mutually cyclic phi nodes like:
@@ -1476,11 +1483,8 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) {
}
}
- // If the phi is within a phi web, which is formed by the def-use chain
- // of phis and all the phis in the web are only used in the other phis. In
- // this case, these phis are dead. We can break the web by removing PN.
- if (isDeadPHIWeb(&PN))
- return replaceInstUsesWith(PN, PoisonValue::get(PN.getType()));
+ if (foldDeadPhiWeb(PN))
+ return nullptr;
// Optimization when the phi only has one use
if (PN.hasOneUse()) {
More information about the llvm-commits
mailing list