[llvm] r178845 - Fix bug in PEI's virtual-register scavenging
Arnold Schwaighofer
aschwaighofer at apple.com
Fri Apr 5 14:07:02 PDT 2013
Hal this commit appears to have broken on of our LTO builds on spec/mesa. I get a segfault somewhere in the prolog epilog inserter that disappears if i revert this patch.
Can you please revert this commit and I'll try to come up with a reduced test case in the meantime.
On Apr 5, 2013, at 12:01 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> Author: hfinkel
> Date: Fri Apr 5 00:01:13 2013
> New Revision: 178845
>
> URL: http://llvm.org/viewvc/llvm-project?rev=178845&view=rev
> Log:
> Fix bug in PEI's virtual-register scavenging
>
> 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.
>
> Modified:
> llvm/trunk/include/llvm/CodeGen/RegisterScavenging.h
> llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp
> llvm/trunk/lib/CodeGen/RegisterScavenging.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/RegisterScavenging.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/RegisterScavenging.h?rev=178845&r1=178844&r2=178845&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/RegisterScavenging.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/RegisterScavenging.h Fri Apr 5 00:01:13 2013
> @@ -93,6 +93,15 @@ 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))
> @@ -100,6 +109,10 @@ public:
> MBBI = I;
> }
>
> + MachineBasicBlock::iterator getCurrentPosition() const {
> + return MBBI;
> + }
> +
> /// getRegsUsed - return all registers currently in use in used.
> void getRegsUsed(BitVector &used, bool includeReserved);
>
> @@ -171,6 +184,10 @@ 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);
>
>
> Modified: llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp?rev=178845&r1=178844&r2=178845&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp (original)
> +++ llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp Fri Apr 5 00:01:13 2013
> @@ -826,6 +826,8 @@ void PEI::scavengeFrameVirtualRegs(Machi
> 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
> @@ -869,8 +871,20 @@ void PEI::scavengeFrameVirtualRegs(Machi
> // 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++);
> - RS->skipTo(I == BB->begin() ? NULL : llvm::prior(I));
> + 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));
> } else
> ++I;
> }
>
> Modified: llvm/trunk/lib/CodeGen/RegisterScavenging.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegisterScavenging.cpp?rev=178845&r1=178844&r2=178845&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/RegisterScavenging.cpp (original)
> +++ llvm/trunk/lib/CodeGen/RegisterScavenging.cpp Fri Apr 5 00:01:13 2013
> @@ -110,30 +110,11 @@ void RegScavenger::addRegWithSubRegs(Bit
> BV.set(*SubRegs);
> }
>
> -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!");
> +void RegScavenger::determineKillsAndDefs() {
> + assert(Tracking && "Must be tracking to determine kills and defs");
>
> MachineInstr *MI = MBBI;
> -
> - for (SmallVector<ScavengedInfo, 2>::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;
> + assert(!MI->isDebugValue() && "Debug values have no kills or defs");
>
> // Find out which registers are early clobbered, killed, defined, and marked
> // def-dead in this instruction.
> @@ -167,6 +148,54 @@ void RegScavenger::forward() {
> 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<ScavengedInfo, 2>::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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list