From c520b90745c14d27ef5fcb163afb65f09ff0d70f Mon Sep 17 00:00:00 2001 From: chengjunp Date: Mon, 16 Sep 2024 20:22:44 +0000 Subject: [PATCH 1/7] 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 bcff9a72b6572..697e7b35033cd 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 &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(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 Stack; + SmallPtrSet 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(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(PN.user_back()); - if (PHINode *PU = dyn_cast(PHIUser)) { - SmallPtrSet 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(PN.user_back()); if (PHIUser->hasOneUse() && (isa(PHIUser) || isa(PHIUser) || isa(PHIUser)) && diff --git a/llvm/test/Transforms/InstCombine/phi.ll b/llvm/test/Transforms/InstCombine/phi.ll index 3b1fa3a97d9cd..b33ad9a7d339f 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 Date: Mon, 16 Sep 2024 20:38:53 +0000 Subject: [PATCH 2/7] 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 697e7b35033cd..094c74a207eb1 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(Use); - if (!PhiUse) + if (PHINode *PhiUse = dyn_cast(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 Date: Wed, 18 Sep 2024 00:08:27 +0000 Subject: [PATCH 3/7] 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 a051a568bfd62..5e567c738f775 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 094c74a207eb1..8ed3be6bc4a3c 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 Stack; + SmallPtrSet 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(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 Stack; - SmallPtrSet 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(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()) { From 4676f293db666cf77b9a2588f2db29a230602e98 Mon Sep 17 00:00:00 2001 From: chengjunp Date: Wed, 18 Sep 2024 00:16:16 +0000 Subject: [PATCH 4/7] Format --- llvm/lib/Transforms/InstCombine/InstCombineInternal.h | 2 +- llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h index 5e567c738f775..a2c45475e5809 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h +++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h @@ -635,7 +635,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final 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. + // 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); diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp index 8ed3be6bc4a3c..347db2d5dd6e0 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -54,9 +54,9 @@ 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. +// 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){ +bool InstCombinerImpl::foldDeadPhiWeb(PHINode &PN) { SmallVector Stack; SmallPtrSet Visited; Stack.push_back(&PN); @@ -1004,7 +1004,6 @@ Instruction *InstCombinerImpl::foldPHIArgOpIntoPHI(PHINode &PN) { return NewCI; } - /// Return true if this phi node is always equal to NonPhiInVal. /// This happens with mutually cyclic phi nodes like: /// z = some value; x = phi (y, z); y = phi (x, z) From d71fb850f44ace91c2181b5095f48eeca734bf56 Mon Sep 17 00:00:00 2001 From: Chengjunp Date: Tue, 17 Sep 2024 17:21:04 -0700 Subject: [PATCH 5/7] Update llvm/lib/Transforms/InstCombine/InstCombineInternal.h Co-authored-by: Yingwei Zheng --- llvm/lib/Transforms/InstCombine/InstCombineInternal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h index a2c45475e5809..fd07b13c32ffb 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h +++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h @@ -634,7 +634,7 @@ 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 + /// 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); From 93b9fd932587b1dfa79e8cab30ac91971d68230f Mon Sep 17 00:00:00 2001 From: Chengjunp Date: Tue, 17 Sep 2024 17:21:20 -0700 Subject: [PATCH 6/7] Update llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp Co-authored-by: Yingwei Zheng --- 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 347db2d5dd6e0..3fd54b7b50f4a 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -53,7 +53,7 @@ void InstCombinerImpl::PHIArgMergedDebugLoc(Instruction *Inst, PHINode &PN) { } } -// If the phi is within a phi web, which is formed by the def-use chain +/// 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) { From 95c916b8e1f0ba4195732e7d45a7f8c9e54110bb Mon Sep 17 00:00:00 2001 From: chengjunp Date: Wed, 18 Sep 2024 00:25:56 +0000 Subject: [PATCH 7/7] Update the comments --- llvm/lib/Transforms/InstCombine/InstCombineInternal.h | 4 ++-- llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h index fd07b13c32ffb..da6f991ad4cd1 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h +++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h @@ -635,8 +635,8 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final 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. + /// 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, diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp index 3fd54b7b50f4a..cb5c447305126 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -54,8 +54,8 @@ 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. +/// 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 Stack; SmallPtrSet Visited;