From 8c494ab759266322aa05d5e99af9c05eb0d44576 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Fri, 27 Oct 2006 23:50:33 +0000 Subject: [PATCH] Fix a bug in merged condition handling (CodeGen/Generic/2006-10-27-CondFolding.ll). Add many fewer CFG edges and PHI node entries. If there is a switch which has the same block as multiple destinations, only add that block once as a successor/phi node (in the jumptable case) git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@31242 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | 177 +++++++++++------- 1 file changed, 110 insertions(+), 67 deletions(-) diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp index 0295fc3b78b..397d430ceab 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp @@ -282,24 +282,24 @@ FunctionLoweringInfo::FunctionLoweringInfo(TargetLowering &tli, // Create Machine PHI nodes for LLVM PHI nodes, lowering them as // appropriate. PHINode *PN; - for (BasicBlock::iterator I = BB->begin(); - (PN = dyn_cast(I)); ++I) - if (!PN->use_empty()) { - MVT::ValueType VT = TLI.getValueType(PN->getType()); - unsigned NumElements; - if (VT != MVT::Vector) - NumElements = TLI.getNumElements(VT); - else { - MVT::ValueType VT1,VT2; - NumElements = - TLI.getPackedTypeBreakdown(cast(PN->getType()), - VT1, VT2); - } - unsigned PHIReg = ValueMap[PN]; - assert(PHIReg &&"PHI node does not have an assigned virtual register!"); - for (unsigned i = 0; i != NumElements; ++i) - BuildMI(MBB, TargetInstrInfo::PHI, PN->getNumOperands(), PHIReg+i); + for (BasicBlock::iterator I = BB->begin();(PN = dyn_cast(I)); ++I){ + if (PN->use_empty()) continue; + + MVT::ValueType VT = TLI.getValueType(PN->getType()); + unsigned NumElements; + if (VT != MVT::Vector) + NumElements = TLI.getNumElements(VT); + else { + MVT::ValueType VT1,VT2; + NumElements = + TLI.getPackedTypeBreakdown(cast(PN->getType()), + VT1, VT2); } + unsigned PHIReg = ValueMap[PN]; + assert(PHIReg && "PHI node does not have an assigned virtual register!"); + for (unsigned i = 0; i != NumElements; ++i) + BuildMI(MBB, TargetInstrInfo::PHI, PN->getNumOperands(), PHIReg+i); + } } } @@ -497,6 +497,7 @@ public: void FindMergedConditions(Value *Cond, MachineBasicBlock *TBB, MachineBasicBlock *FBB, MachineBasicBlock *CurBB, unsigned Opc); + bool isExportableFromCurrentBlock(Value *V, const BasicBlock *FromBB); void ExportFromCurrentBlock(Value *V); // Terminator instructions. @@ -798,15 +799,39 @@ void SelectionDAGLowering::ExportFromCurrentBlock(Value *V) { PendingLoads.push_back(CopyValueToVirtualRegister(V, Reg)); } +bool SelectionDAGLowering::isExportableFromCurrentBlock(Value *V, + const BasicBlock *FromBB) { + // The operands of the setcc have to be in this block. We don't know + // how to export them from some other block. + if (Instruction *VI = dyn_cast(V)) { + // Can export from current BB. + if (VI->getParent() == FromBB) + return true; + + // Is already exported, noop. + return FuncInfo.isExportedInst(V); + } + + // If this is an argument, we can export it if the BB is the entry block or + // if it is already exported. + if (isa(V)) { + if (FromBB == &FromBB->getParent()->getEntryBlock()) + return true; + + // Otherwise, can only export this if it is already exported. + return FuncInfo.isExportedInst(V); + } + + // Otherwise, constants can always be exported. + return true; +} + /// FindMergedConditions - If Cond is an expression like void SelectionDAGLowering::FindMergedConditions(Value *Cond, MachineBasicBlock *TBB, MachineBasicBlock *FBB, MachineBasicBlock *CurBB, unsigned Opc) { - // FIXME: HANDLE AND. - // FIXME: HANDLE NOT - // If this node is not part of the or/and tree, emit it as a branch. BinaryOperator *BOp = dyn_cast(Cond); @@ -819,12 +844,8 @@ void SelectionDAGLowering::FindMergedConditions(Value *Cond, if (BOp && isa(BOp) && // The operands of the setcc have to be in this block. We don't know // how to export them from some other block. - (!isa(BOp->getOperand(0)) || - cast(BOp->getOperand(0))->getParent() == BB || - FuncInfo.isExportedInst(BOp->getOperand(0))) && - (!isa(BOp->getOperand(1)) || - cast(BOp->getOperand(1))->getParent() == BB || - FuncInfo.isExportedInst(BOp->getOperand(1)))) { + isExportableFromCurrentBlock(BOp->getOperand(0), BB) && + isExportableFromCurrentBlock(BOp->getOperand(1), BB)) { ExportFromCurrentBlock(BOp->getOperand(0)); ExportFromCurrentBlock(BOp->getOperand(1)); @@ -1222,10 +1243,18 @@ void SelectionDAGLowering::visitSwitch(SwitchInst &I) { DestBBs.push_back(Default); } - // Update successor info + // Update successor info. Add one edge to each unique successor. + // Vector bool would be better, but vector is really slow. + std::vector SuccsHandled; + SuccsHandled.resize(CurMBB->getParent()->getNumBlockIDs()); + for (std::vector::iterator I = DestBBs.begin(), - E = DestBBs.end(); I != E; ++I) - JumpTableBB->addSuccessor(*I); + E = DestBBs.end(); I != E; ++I) { + if (!SuccsHandled[(*I)->getNumber()]) { + SuccsHandled[(*I)->getNumber()] = true; + JumpTableBB->addSuccessor(*I); + } + } // Create a jump table index for this jump table, or return an existing // one. @@ -3710,63 +3739,77 @@ void SelectionDAGISel::BuildSelectionDAG(SelectionDAG &DAG, BasicBlock *LLVMBB, // BB. As such, the start of the BB might correspond to a different MBB than // the end. // + TerminatorInst *TI = LLVMBB->getTerminator(); // Emit constants only once even if used by multiple PHI nodes. std::map ConstantsOut; + // Vector bool would be better, but vector is really slow. + std::vector SuccsHandled; + if (TI->getNumSuccessors()) + SuccsHandled.resize(BB->getParent()->getNumBlockIDs()); + // Check successor nodes PHI nodes that expect a constant to be available from // this block. - TerminatorInst *TI = LLVMBB->getTerminator(); for (unsigned succ = 0, e = TI->getNumSuccessors(); succ != e; ++succ) { BasicBlock *SuccBB = TI->getSuccessor(succ); if (!isa(SuccBB->begin())) continue; + MachineBasicBlock *SuccMBB = FuncInfo.MBBMap[SuccBB]; - MachineBasicBlock::iterator MBBI = FuncInfo.MBBMap[SuccBB]->begin(); + // If this terminator has multiple identical successors (common for + // switches), only handle each succ once. + unsigned SuccMBBNo = SuccMBB->getNumber(); + if (SuccsHandled[SuccMBBNo]) continue; + SuccsHandled[SuccMBBNo] = true; + + MachineBasicBlock::iterator MBBI = SuccMBB->begin(); PHINode *PN; // At this point we know that there is a 1-1 correspondence between LLVM PHI // nodes and Machine PHI nodes, but the incoming operands have not been // emitted yet. for (BasicBlock::iterator I = SuccBB->begin(); - (PN = dyn_cast(I)); ++I) - if (!PN->use_empty()) { - unsigned Reg; - Value *PHIOp = PN->getIncomingValueForBlock(LLVMBB); - if (Constant *C = dyn_cast(PHIOp)) { - unsigned &RegOut = ConstantsOut[C]; - if (RegOut == 0) { - RegOut = FuncInfo.CreateRegForValue(C); - UnorderedChains.push_back( - SDL.CopyValueToVirtualRegister(C, RegOut)); - } - Reg = RegOut; - } else { - Reg = FuncInfo.ValueMap[PHIOp]; - if (Reg == 0) { - assert(isa(PHIOp) && - FuncInfo.StaticAllocaMap.count(cast(PHIOp)) && - "Didn't codegen value into a register!??"); - Reg = FuncInfo.CreateRegForValue(PHIOp); - UnorderedChains.push_back( - SDL.CopyValueToVirtualRegister(PHIOp, Reg)); - } + (PN = dyn_cast(I)); ++I) { + // Ignore dead phi's. + if (PN->use_empty()) continue; + + unsigned Reg; + Value *PHIOp = PN->getIncomingValueForBlock(LLVMBB); + if (Constant *C = dyn_cast(PHIOp)) { + unsigned &RegOut = ConstantsOut[C]; + if (RegOut == 0) { + RegOut = FuncInfo.CreateRegForValue(C); + UnorderedChains.push_back( + SDL.CopyValueToVirtualRegister(C, RegOut)); } - - // Remember that this register needs to added to the machine PHI node as - // the input for this MBB. - MVT::ValueType VT = TLI.getValueType(PN->getType()); - unsigned NumElements; - if (VT != MVT::Vector) - NumElements = TLI.getNumElements(VT); - else { - MVT::ValueType VT1,VT2; - NumElements = - TLI.getPackedTypeBreakdown(cast(PN->getType()), - VT1, VT2); + Reg = RegOut; + } else { + Reg = FuncInfo.ValueMap[PHIOp]; + if (Reg == 0) { + assert(isa(PHIOp) && + FuncInfo.StaticAllocaMap.count(cast(PHIOp)) && + "Didn't codegen value into a register!??"); + Reg = FuncInfo.CreateRegForValue(PHIOp); + UnorderedChains.push_back( + SDL.CopyValueToVirtualRegister(PHIOp, Reg)); } - for (unsigned i = 0, e = NumElements; i != e; ++i) - PHINodesToUpdate.push_back(std::make_pair(MBBI++, Reg+i)); } + + // Remember that this register needs to added to the machine PHI node as + // the input for this MBB. + MVT::ValueType VT = TLI.getValueType(PN->getType()); + unsigned NumElements; + if (VT != MVT::Vector) + NumElements = TLI.getNumElements(VT); + else { + MVT::ValueType VT1,VT2; + NumElements = + TLI.getPackedTypeBreakdown(cast(PN->getType()), + VT1, VT2); + } + for (unsigned i = 0, e = NumElements; i != e; ++i) + PHINodesToUpdate.push_back(std::make_pair(MBBI++, Reg+i)); + } } ConstantsOut.clear();