diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h index a051a568bfd62..da6f991ad4cd1 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 bcff9a72b6572..cb5c447305126 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,26 +1004,6 @@ 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 always equal to NonPhiInVal. /// This happens with mutually cyclic phi nodes like: /// z = some value; x = phi (y, z); y = phi (x, z) @@ -1474,27 +1482,21 @@ 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 (foldDeadPhiWeb(PN)) + return nullptr; + + // 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 +}