From f13bbf1d5891272583e803ebce798b3dbdacc353 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 26 Mar 2018 16:23:29 +0000 Subject: [Pipeliner] Fix assert caused by pipeliner serialization The pipeliner is asserting because the serialization step that occurs at the end is deleting an instruction. The assert occurs later on because there is a use without a definition. The problem occurs when an instruction defines a value used by a REQ_SEQUENCE and that value is used by a COPY instruction. The latencies between these instructions are zero, so they are put in to the same packet. The serialization code is unable to handle this correctly, and ends up putting the REG_SEQUENCE before its definition. There is special code in the serialization step that attempts to handle zero-cost instructions (phis, copy, reg_sequence) differently than regular instructions. Unfortunately, this means the order does not come out correct. This patch simplifies the code by changing the seperate steps for handling zero-cost and regular instructions. Only phis are handled separate now, since they should occurs first. Then, this patch adds checks to make use the MoveUse is set to the smallest value if there are multiple uses in a cycle. Patch by Brendon Cahoon. llvm-svn: 328540 --- llvm/lib/CodeGen/MachinePipeliner.cpp | 51 ++++++++++++++++------------------- 1 file changed, 23 insertions(+), 28 deletions(-) (limited to 'llvm/lib/CodeGen/MachinePipeliner.cpp') diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp index 78a34f0..c50805f 100644 --- a/llvm/lib/CodeGen/MachinePipeliner.cpp +++ b/llvm/lib/CodeGen/MachinePipeliner.cpp @@ -702,7 +702,7 @@ public: bool isValidSchedule(SwingSchedulerDAG *SSD); void finalizeSchedule(SwingSchedulerDAG *SSD); - bool orderDependence(SwingSchedulerDAG *SSD, SUnit *SU, + void orderDependence(SwingSchedulerDAG *SSD, SUnit *SU, std::deque &Insts); bool isLoopCarried(SwingSchedulerDAG *SSD, MachineInstr &Phi); bool isLoopCarriedDefOfUse(SwingSchedulerDAG *SSD, MachineInstr *Inst, @@ -3745,7 +3745,7 @@ void SMSchedule::computeStart(SUnit *SU, int *MaxEarlyStart, int *MinLateStart, /// Order the instructions within a cycle so that the definitions occur /// before the uses. Returns true if the instruction is added to the start /// of the list, or false if added to the end. -bool SMSchedule::orderDependence(SwingSchedulerDAG *SSD, SUnit *SU, +void SMSchedule::orderDependence(SwingSchedulerDAG *SSD, SUnit *SU, std::deque &Insts) { MachineInstr *MI = SU->getInstr(); bool OrderBeforeUse = false; @@ -3758,13 +3758,11 @@ bool SMSchedule::orderDependence(SwingSchedulerDAG *SSD, SUnit *SU, unsigned Pos = 0; for (std::deque::iterator I = Insts.begin(), E = Insts.end(); I != E; ++I, ++Pos) { - // Relative order of Phis does not matter. - if (MI->isPHI() && (*I)->getInstr()->isPHI()) - continue; for (unsigned i = 0, e = MI->getNumOperands(); i < e; ++i) { MachineOperand &MO = MI->getOperand(i); if (!MO.isReg() || !TargetRegisterInfo::isVirtualRegister(MO.getReg())) continue; + unsigned Reg = MO.getReg(); unsigned BasePos, OffsetPos; if (ST.getInstrInfo()->getBaseAndOffsetPosition(*MI, BasePos, OffsetPos)) @@ -3776,7 +3774,8 @@ bool SMSchedule::orderDependence(SwingSchedulerDAG *SSD, SUnit *SU, (*I)->getInstr()->readsWritesVirtualRegister(Reg); if (MO.isDef() && Reads && stageScheduled(*I) <= StageInst1) { OrderBeforeUse = true; - MoveUse = Pos; + if (MoveUse == 0) + MoveUse = Pos; } else if (MO.isDef() && Reads && stageScheduled(*I) > StageInst1) { // Add the instruction after the scheduled instruction. OrderAfterDef = true; @@ -3784,14 +3783,16 @@ bool SMSchedule::orderDependence(SwingSchedulerDAG *SSD, SUnit *SU, } else if (MO.isUse() && Writes && stageScheduled(*I) == StageInst1) { if (cycleScheduled(*I) == cycleScheduled(SU) && !(*I)->isSucc(SU)) { OrderBeforeUse = true; - MoveUse = Pos; + if (MoveUse == 0) + MoveUse = Pos; } else { OrderAfterDef = true; MoveDef = Pos; } } else if (MO.isUse() && Writes && stageScheduled(*I) > StageInst1) { OrderBeforeUse = true; - MoveUse = Pos; + if (MoveUse == 0) + MoveUse = Pos; if (MoveUse != 0) { OrderAfterDef = true; MoveDef = Pos - 1; @@ -3799,11 +3800,14 @@ bool SMSchedule::orderDependence(SwingSchedulerDAG *SSD, SUnit *SU, } else if (MO.isUse() && Writes && stageScheduled(*I) < StageInst1) { // Add the instruction before the scheduled instruction. OrderBeforeUse = true; - MoveUse = Pos; + if (MoveUse == 0) + MoveUse = Pos; } else if (MO.isUse() && stageScheduled(*I) == StageInst1 && isLoopCarriedDefOfUse(SSD, (*I)->getInstr(), MO)) { - OrderBeforeDef = true; - MoveUse = Pos; + if (MoveUse == 0) { + OrderBeforeDef = true; + MoveUse = Pos; + } } } // Check for order dependences between instructions. Make sure the source @@ -3848,16 +3852,10 @@ bool SMSchedule::orderDependence(SwingSchedulerDAG *SSD, SUnit *SU, Insts.erase(Insts.begin() + MoveDef); Insts.erase(Insts.begin() + MoveUse); } - if (orderDependence(SSD, UseSU, Insts)) { - Insts.push_front(SU); - orderDependence(SSD, DefSU, Insts); - return true; - } - Insts.pop_back(); - Insts.push_back(SU); - Insts.push_back(UseSU); + orderDependence(SSD, UseSU, Insts); + orderDependence(SSD, SU, Insts); orderDependence(SSD, DefSU, Insts); - return false; + return; } // Put the new instruction first if there is a use in the list. Otherwise, // put it at the end of the list. @@ -3865,7 +3863,6 @@ bool SMSchedule::orderDependence(SwingSchedulerDAG *SSD, SUnit *SU, Insts.push_front(SU); else Insts.push_back(SU); - return OrderBeforeUse; } /// Return true if the scheduled Phi has a loop carried operand. @@ -4151,22 +4148,20 @@ void SMSchedule::finalizeSchedule(SwingSchedulerDAG *SSD) { // generated code. for (int Cycle = getFirstCycle(), E = getFinalCycle(); Cycle <= E; ++Cycle) { std::deque &cycleInstrs = ScheduledInstrs[Cycle]; - std::deque newOrderZC; - // Put the zero-cost, pseudo instructions at the start of the cycle. + std::deque newOrderPhi; for (unsigned i = 0, e = cycleInstrs.size(); i < e; ++i) { SUnit *SU = cycleInstrs[i]; - if (ST.getInstrInfo()->isZeroCost(SU->getInstr()->getOpcode())) - orderDependence(SSD, SU, newOrderZC); + if (SU->getInstr()->isPHI()) + newOrderPhi.push_back(SU); } std::deque newOrderI; - // Then, add the regular instructions back. for (unsigned i = 0, e = cycleInstrs.size(); i < e; ++i) { SUnit *SU = cycleInstrs[i]; - if (!ST.getInstrInfo()->isZeroCost(SU->getInstr()->getOpcode())) + if (!SU->getInstr()->isPHI()) orderDependence(SSD, SU, newOrderI); } // Replace the old order with the new order. - cycleInstrs.swap(newOrderZC); + cycleInstrs.swap(newOrderPhi); cycleInstrs.insert(cycleInstrs.end(), newOrderI.begin(), newOrderI.end()); SSD->fixupRegisterOverlaps(cycleInstrs); } -- cgit v1.1