From 2aa2fdeed9dc8af922e424545eae56d81c10599f Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Fri, 23 Jul 2021 22:29:04 +0300 Subject: [PATCH] [NFC][BasicBlockUtils] Refactor GetIfCondition() to return the branch, not it's condition Otherwise e.g. the FoldTwoEntryPHINode() has to do a lot of legwork to re-deduce what is the dominant block (i.e. for which block is this branch the terminator). --- llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h | 6 +++--- llvm/lib/Transforms/Utils/BasicBlockUtils.cpp | 8 ++++---- llvm/lib/Transforms/Utils/FlattenCFG.cpp | 16 ++++++++++------ llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 10 ++++++---- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h index c78e964..b45c182 100644 --- a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h +++ b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h @@ -487,15 +487,15 @@ void SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore, MDNode *BranchWeights = nullptr); /// Check whether BB is the merge point of a if-region. -/// If so, return the boolean condition that determines which entry into +/// If so, return the branch instruction that determines which entry into /// BB will be taken. Also, return by references the block that will be /// entered from if the condition is true, and the block that will be /// entered if the condition is false. /// /// This does no checking to see if the true/false blocks have large or unsavory /// instructions in them. -Value *GetIfCondition(BasicBlock *BB, BasicBlock *&IfTrue, - BasicBlock *&IfFalse); +BranchInst *GetIfCondition(BasicBlock *BB, BasicBlock *&IfTrue, + BasicBlock *&IfFalse); // Split critical edges where the source of the edge is an indirectbr // instruction. This isn't always possible, but we can handle some easy cases. diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp index def695e..ee933b6 100644 --- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp +++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp @@ -1456,8 +1456,8 @@ void llvm::SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore, ReplaceInstWithInst(HeadOldTerm, HeadNewTerm); } -Value *llvm::GetIfCondition(BasicBlock *BB, BasicBlock *&IfTrue, - BasicBlock *&IfFalse) { +BranchInst *llvm::GetIfCondition(BasicBlock *BB, BasicBlock *&IfTrue, + BasicBlock *&IfFalse) { PHINode *SomePHI = dyn_cast(BB->begin()); BasicBlock *Pred1 = nullptr; BasicBlock *Pred2 = nullptr; @@ -1523,7 +1523,7 @@ Value *llvm::GetIfCondition(BasicBlock *BB, BasicBlock *&IfTrue, return nullptr; } - return Pred1Br->getCondition(); + return Pred1Br; } // Ok, if we got here, both predecessors end with an unconditional branch to @@ -1545,7 +1545,7 @@ Value *llvm::GetIfCondition(BasicBlock *BB, BasicBlock *&IfTrue, IfTrue = Pred2; IfFalse = Pred1; } - return BI->getCondition(); + return BI; } // After creating a control flow hub, the operands of PHINodes in an outgoing diff --git a/llvm/lib/Transforms/Utils/FlattenCFG.cpp b/llvm/lib/Transforms/Utils/FlattenCFG.cpp index a475f45..dbcacc2 100644 --- a/llvm/lib/Transforms/Utils/FlattenCFG.cpp +++ b/llvm/lib/Transforms/Utils/FlattenCFG.cpp @@ -411,8 +411,10 @@ bool FlattenCFGOpt::CompareIfRegionBlock(BasicBlock *Block1, BasicBlock *Block2, /// approach goes for the opposite case. bool FlattenCFGOpt::MergeIfRegion(BasicBlock *BB, IRBuilder<> &Builder) { BasicBlock *IfTrue2, *IfFalse2; - Value *IfCond2 = GetIfCondition(BB, IfTrue2, IfFalse2); - Instruction *CInst2 = dyn_cast_or_null(IfCond2); + BranchInst *DomBI2 = GetIfCondition(BB, IfTrue2, IfFalse2); + if (!DomBI2) + return false; + Instruction *CInst2 = dyn_cast(DomBI2->getCondition()); if (!CInst2) return false; @@ -421,8 +423,10 @@ bool FlattenCFGOpt::MergeIfRegion(BasicBlock *BB, IRBuilder<> &Builder) { return false; BasicBlock *IfTrue1, *IfFalse1; - Value *IfCond1 = GetIfCondition(SecondEntryBlock, IfTrue1, IfFalse1); - Instruction *CInst1 = dyn_cast_or_null(IfCond1); + BranchInst *DomBI1 = GetIfCondition(SecondEntryBlock, IfTrue1, IfFalse1); + if (!DomBI1) + return false; + Instruction *CInst1 = dyn_cast(DomBI1->getCondition()); if (!CInst1) return false; @@ -479,7 +483,7 @@ bool FlattenCFGOpt::MergeIfRegion(BasicBlock *BB, IRBuilder<> &Builder) { FirstEntryBlock->getInstList() .splice(FirstEntryBlock->end(), SecondEntryBlock->getInstList()); BranchInst *PBI = cast(FirstEntryBlock->getTerminator()); - assert(PBI->getCondition() == IfCond2); + assert(PBI->getCondition() == CInst2); BasicBlock *SaveInsertBB = Builder.GetInsertBlock(); BasicBlock::iterator SaveInsertPt = Builder.GetInsertPoint(); Builder.SetInsertPoint(PBI); @@ -494,7 +498,7 @@ bool FlattenCFGOpt::MergeIfRegion(BasicBlock *BB, IRBuilder<> &Builder) { PBI->swapSuccessors(); } Value *NC = Builder.CreateBinOp(CombineOp, CInst1, CInst2); - PBI->replaceUsesOfWith(IfCond2, NC); + PBI->replaceUsesOfWith(CInst2, NC); Builder.SetInsertPoint(SaveInsertBB, SaveInsertPt); // Handle PHI node to replace its predecessors to FirstEntryBlock. diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 8effe2d..31f83095 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -2708,10 +2708,12 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI, BasicBlock *BB = PN->getParent(); BasicBlock *IfTrue, *IfFalse; - Value *IfCond = GetIfCondition(BB, IfTrue, IfFalse); - if (!IfCond || - // Don't bother if the branch will be constant folded trivially. - isa(IfCond)) + BranchInst *DomBI = GetIfCondition(BB, IfTrue, IfFalse); + if (!DomBI) + return false; + Value *IfCond = DomBI->getCondition(); + // Don't bother if the branch will be constant folded trivially. + if (isa(IfCond)) return false; // Don't try to fold an unreachable block. For example, the phi node itself -- 2.7.4