From b6cfeb63f8192724362565567a4cb4aa642baa1d Mon Sep 17 00:00:00 2001 From: Hal Finkel Date: Fri, 5 Apr 2013 21:30:40 +0000 Subject: [PATCH] Revert r178845 - Fix bug in PEI's virtual-register scavenging Reverting because this breaks one of the LTO builders. Original commit message: This change fixes a bug that I introduced in r178058. After a register is scavenged using one of the available spills slots the instruction defining the virtual register needs to be moved to after the spill code. The scavenger has already processed the defining instruction so that registers killed by that instruction are available for definition in that same instruction. Unfortunately, after this, the scavenger needs to iterate through the spill code and then visit, again, the instruction that defines the now-scavenged register. In order to avoid confusion, the register scavenger needs the ability to 'back up' through the spill code so that it can again process the instructions in the appropriate order. Prior to this fix, once the scavenger reached the just-moved instruction, it would assert if it killed any registers because, having already processed the instruction, it believed they were undefined. Unfortunately, I don't yet have a small test case. Thanks to Pranav Bhandarkar for diagnosing the problem and testing this fix. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@178916 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/RegisterScavenging.h | 17 ------ lib/CodeGen/PrologEpilogInserter.cpp | 18 +----- lib/CodeGen/RegisterScavenging.cpp | 73 +++++++---------------- 3 files changed, 24 insertions(+), 84 deletions(-) diff --git a/include/llvm/CodeGen/RegisterScavenging.h b/include/llvm/CodeGen/RegisterScavenging.h index 95bf29167c2..49d16892f87 100644 --- a/include/llvm/CodeGen/RegisterScavenging.h +++ b/include/llvm/CodeGen/RegisterScavenging.h @@ -93,15 +93,6 @@ public: while (MBBI != I) forward(); } - /// Invert the behavior of forward() on the current instruction (undo the - /// changes to the available registers made by forward()). - void unprocess(); - - /// Unprocess instructions until you reach the provided iterator. - void unprocess(MachineBasicBlock::iterator I) { - while (MBBI != I) unprocess(); - } - /// skipTo - Move the internal MBB iterator but do not update register states. void skipTo(MachineBasicBlock::iterator I) { if (I == MachineBasicBlock::iterator(NULL)) @@ -109,10 +100,6 @@ public: MBBI = I; } - MachineBasicBlock::iterator getCurrentPosition() const { - return MBBI; - } - /// getRegsUsed - return all registers currently in use in used. void getRegsUsed(BitVector &used, bool includeReserved); @@ -184,10 +171,6 @@ private: RegsAvailable |= Regs; } - /// Processes the current instruction and fill the KillRegs and DefRegs bit - /// vectors. - void determineKillsAndDefs(); - /// Add Reg and all its sub-registers to BV. void addRegWithSubRegs(BitVector &BV, unsigned Reg); diff --git a/lib/CodeGen/PrologEpilogInserter.cpp b/lib/CodeGen/PrologEpilogInserter.cpp index e5872df731a..5a168dd244c 100644 --- a/lib/CodeGen/PrologEpilogInserter.cpp +++ b/lib/CodeGen/PrologEpilogInserter.cpp @@ -826,8 +826,6 @@ void PEI::scavengeFrameVirtualRegs(MachineFunction &Fn) { for (MachineBasicBlock::iterator I = BB->begin(); I != BB->end(); ) { MachineInstr *MI = I; MachineBasicBlock::iterator J = llvm::next(I); - MachineBasicBlock::iterator P = I == BB->begin() ? - MachineBasicBlock::iterator(NULL) : llvm::prior(I); // RS should process this instruction before we might scavenge at this // location. This is because we might be replacing a virtual register @@ -871,20 +869,8 @@ void PEI::scavengeFrameVirtualRegs(MachineFunction &Fn) { // problem because we need the spill code before I: Move I to just // prior to J. if (I != llvm::prior(J)) { - BB->splice(J, BB, I); - - // Before we move I, we need to prepare the RS to visit I again. - // Specifically, RS will assert if it sees uses of registers that - // it believes are undefined. Because we have already processed - // register kills in I, when it visits I again, it will believe that - // those registers are undefined. To avoid this situation, unprocess - // the instruction I. - assert(RS->getCurrentPosition() == I && - "The register scavenger has an unexpected position"); - I = P; - RS->unprocess(P); - - // RS->skipTo(I == BB->begin() ? NULL : llvm::prior(I)); + BB->splice(J, BB, I++); + RS->skipTo(I == BB->begin() ? NULL : llvm::prior(I)); } else ++I; } diff --git a/lib/CodeGen/RegisterScavenging.cpp b/lib/CodeGen/RegisterScavenging.cpp index 07ace7a436c..55a66ba5482 100644 --- a/lib/CodeGen/RegisterScavenging.cpp +++ b/lib/CodeGen/RegisterScavenging.cpp @@ -110,11 +110,30 @@ void RegScavenger::addRegWithSubRegs(BitVector &BV, unsigned Reg) { BV.set(*SubRegs); } -void RegScavenger::determineKillsAndDefs() { - assert(Tracking && "Must be tracking to determine kills and defs"); +void RegScavenger::forward() { + // Move ptr forward. + if (!Tracking) { + MBBI = MBB->begin(); + Tracking = true; + } else { + assert(MBBI != MBB->end() && "Already past the end of the basic block!"); + MBBI = llvm::next(MBBI); + } + assert(MBBI != MBB->end() && "Already at the end of the basic block!"); MachineInstr *MI = MBBI; - assert(!MI->isDebugValue() && "Debug values have no kills or defs"); + + for (SmallVector::iterator I = Scavenged.begin(), + IE = Scavenged.end(); I != IE; ++I) { + if (I->Restore != MI) + continue; + + I->Reg = 0; + I->Restore = NULL; + } + + if (MI->isDebugValue()) + return; // Find out which registers are early clobbered, killed, defined, and marked // def-dead in this instruction. @@ -148,54 +167,6 @@ void RegScavenger::determineKillsAndDefs() { addRegWithSubRegs(DefRegs, Reg); } } -} - -void RegScavenger::unprocess() { - assert(Tracking && "Cannot unprocess because we're not tracking"); - - MachineInstr *MI = MBBI; - if (MI->isDebugValue()) - return; - - determineKillsAndDefs(); - - // Commit the changes. - setUsed(KillRegs); - setUnused(DefRegs); - - if (MBBI == MBB->begin()) { - MBBI = MachineBasicBlock::iterator(NULL); - Tracking = false; - } else - --MBBI; -} - -void RegScavenger::forward() { - // Move ptr forward. - if (!Tracking) { - MBBI = MBB->begin(); - Tracking = true; - } else { - assert(MBBI != MBB->end() && "Already past the end of the basic block!"); - MBBI = llvm::next(MBBI); - } - assert(MBBI != MBB->end() && "Already at the end of the basic block!"); - - MachineInstr *MI = MBBI; - - for (SmallVector::iterator I = Scavenged.begin(), - IE = Scavenged.end(); I != IE; ++I) { - if (I->Restore != MI) - continue; - - I->Reg = 0; - I->Restore = NULL; - } - - if (MI->isDebugValue()) - return; - - determineKillsAndDefs(); // Verify uses and defs. #ifndef NDEBUG -- 2.34.1