[llvm-commits] [llvm] r91549 - in /llvm/trunk/lib/CodeGen: LiveIntervalAnalysis.cpp PHIElimination.cpp PHIElimination.h
Chris Lattner
clattner at apple.com
Wed Dec 16 16:16:14 PST 2009
On Dec 16, 2009, at 10:55 AM, Jakob Stoklund Olesen wrote:
> Author: stoklund
> Date: Wed Dec 16 12:55:53 2009
> New Revision: 91549
>
> URL: http://llvm.org/viewvc/llvm-project?rev=91549&view=rev
> Log:
> Reuse lowered phi nodes.
>
> Tail duplication produces lots of identical phi nodes in different
> basic
> blocks. Teach PHIElimination to reuse the join registers when
> lowering a phi
> node that is identical to an already lowered node. This saves virtual
> registers, and more importantly it avoids creating copies the the
> coalescer
> doesn't know how to eliminate.
Hi Jakob,
I don't have any objection to this patch, but would it alternatively
(or also) make sense for the SSAUpdate engine in taildupe to reuse PHI
nodes when they already exist, instead of always inserting new ones?
Would that help with this issue?
-Chris
>
> Teach LiveIntervalAnalysis about the phi joins with multiple uses.
>
> This patch significantly reduces code size produced by -pre-regalloc-
> taildup.
>
> Modified:
> llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp
> llvm/trunk/lib/CodeGen/PHIElimination.cpp
> llvm/trunk/lib/CodeGen/PHIElimination.h
>
> Modified: llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp?rev=91549&r1=91548&r2=91549&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp (original)
> +++ llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp Wed Dec 16
> 12:55:53 2009
> @@ -415,19 +415,32 @@
> // first redefinition of the vreg that we have seen, go back
> and change
> // the live range in the PHI block to be a different value
> number.
> if (interval.containsOneValue()) {
> - // Remove the old range that we now know has an incorrect
> number.
> +
> VNInfo *VNI = interval.getValNumInfo(0);
> - MachineInstr *Killer = vi.Kills[0];
> - SlotIndex Start = getMBBStartIdx(Killer->getParent());
> - SlotIndex End = getInstructionIndex(Killer).getDefIndex();
> - DEBUG({
> - errs() << " Removing [" << Start << "," << End << "]
> from: ";
> - interval.print(errs(), tri_);
> - errs() << "\n";
> - });
> - interval.removeRange(Start, End);
> - assert(interval.ranges.size() == 1 &&
> - "Newly discovered PHI interval has >1 ranges.");
> + // Phi elimination may have reused the register for
> multiple identical
> + // phi nodes. There will be a kill per phi. Remove the old
> ranges that
> + // we now know have an incorrect number.
> + for (unsigned ki=0, ke=vi.Kills.size(); ki != ke; ++ki) {
> + MachineInstr *Killer = vi.Kills[ki];
> + SlotIndex Start = getMBBStartIdx(Killer->getParent());
> + SlotIndex End = getInstructionIndex(Killer).getDefIndex();
> + DEBUG({
> + errs() << "\n\t\trenaming [" << Start << "," << End
> << "] in: ";
> + interval.print(errs(), tri_);
> + });
> + interval.removeRange(Start, End);
> +
> + // Replace the interval with one of a NEW value number.
> Note that
> + // this value number isn't actually defined by an
> instruction, weird
> + // huh? :)
> + LiveRange LR(Start, End,
> + interval.getNextValue(SlotIndex(Start, true),
> + 0, false,
> VNInfoAllocator));
> + LR.valno->setIsPHIDef(true);
> + interval.addRange(LR);
> + LR.valno->addKill(End);
> + }
> +
> MachineBasicBlock *killMBB = getMBBFromIndex(VNI->def);
> VNI->addKill(indexes_->getTerminatorGap(killMBB));
> VNI->setHasPHIKill(true);
> @@ -435,20 +448,6 @@
> errs() << " RESULT: ";
> interval.print(errs(), tri_);
> });
> -
> - // Replace the interval with one of a NEW value number.
> Note that this
> - // value number isn't actually defined by an instruction,
> weird huh? :)
> - LiveRange LR(Start, End,
> -
> interval.getNextValue(SlotIndex(getMBBStartIdx(Killer->getParent()),
> true),
> - 0, false, VNInfoAllocator));
> - LR.valno->setIsPHIDef(true);
> - DEBUG(errs() << " replace range with " << LR);
> - interval.addRange(LR);
> - LR.valno->addKill(End);
> - DEBUG({
> - errs() << " RESULT: ";
> - interval.print(errs(), tri_);
> - });
> }
>
> // In the case of PHI elimination, each variable definition is
> only
>
> Modified: llvm/trunk/lib/CodeGen/PHIElimination.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PHIElimination.cpp?rev=91549&r1=91548&r2=91549&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/CodeGen/PHIElimination.cpp (original)
> +++ llvm/trunk/lib/CodeGen/PHIElimination.cpp Wed Dec 16 12:55:53 2009
> @@ -35,6 +35,7 @@
>
> STATISTIC(NumAtomic, "Number of atomic phis lowered");
> STATISTIC(NumSplits, "Number of critical edges split on demand");
> +STATISTIC(NumReused, "Number of reused lowered phis");
>
> char PHIElimination::ID = 0;
> static RegisterPass<PHIElimination>
> @@ -78,6 +79,12 @@
> DefMI->eraseFromParent();
> }
>
> + // Clean up the lowered PHI instructions.
> + for (LoweredPHIMap::iterator I = LoweredPHIs.begin(), E =
> LoweredPHIs.end();
> + I != E; ++I)
> + Fn.DeleteMachineInstr(I->first);
> + LoweredPHIs.clear();
> +
> ImpDefs.clear();
> VRegPHIUseCount.clear();
> return Changed;
> @@ -168,6 +175,7 @@
> void llvm::PHIElimination::LowerAtomicPHINode(
> MachineBasicBlock &MBB,
> MachineBasicBlock::iterator
> AfterPHIsIt) {
> + ++NumAtomic;
> // Unlink the PHI node from the basic block, but don't delete the
> PHI yet.
> MachineInstr *MPhi = MBB.remove(MBB.begin());
>
> @@ -179,6 +187,7 @@
> MachineFunction &MF = *MBB.getParent();
> const TargetRegisterClass *RC =
> MF.getRegInfo().getRegClass(DestReg);
> unsigned IncomingReg = 0;
> + bool reusedIncoming = false; // Is IncomingReg reused from an
> earlier PHI?
>
> // Insert a register to register copy at the top of the current
> block (but
> // after any remaining phi nodes) which copies the new incoming
> register
> @@ -190,7 +199,18 @@
> BuildMI(MBB, AfterPHIsIt, MPhi->getDebugLoc(),
> TII->get(TargetInstrInfo::IMPLICIT_DEF), DestReg);
> else {
> - IncomingReg = MF.getRegInfo().createVirtualRegister(RC);
> + // Can we reuse an earlier PHI node? This only happens for
> critical edges,
> + // typically those created by tail duplication.
> + unsigned &entry = LoweredPHIs[MPhi];
> + if (entry) {
> + // An identical PHI node was already lowered. Reuse the
> incoming register.
> + IncomingReg = entry;
> + reusedIncoming = true;
> + ++NumReused;
> + DEBUG(errs() << "Reusing %reg" << IncomingReg << " for " <<
> *MPhi);
> + } else {
> + entry = IncomingReg =
> MF.getRegInfo().createVirtualRegister(RC);
> + }
> TII->copyRegToReg(MBB, AfterPHIsIt, DestReg, IncomingReg, RC, RC);
> }
>
> @@ -204,8 +224,20 @@
> MachineInstr *PHICopy = prior(AfterPHIsIt);
>
> if (IncomingReg) {
> + LiveVariables::VarInfo &VI = LV->getVarInfo(IncomingReg);
> +
> // Increment use count of the newly created virtual register.
> - LV->getVarInfo(IncomingReg).NumUses++;
> + VI.NumUses++;
> +
> + // When we are reusing the incoming register, it may already
> have been
> + // killed in this block. The old kill will also have been
> inserted at
> + // AfterPHIsIt, so it appears before the current PHICopy.
> + if (reusedIncoming)
> + if (MachineInstr *OldKill = VI.findKill(&MBB)) {
> + DEBUG(errs() << "Remove old kill from " << *OldKill);
> + LV->removeVirtualRegisterKilled(IncomingReg, OldKill);
> + DEBUG(MBB.dump());
> + }
>
> // Add information to LiveVariables to know that the incoming
> value is
> // killed. Note that because the value is defined in several
> places (once
> @@ -228,7 +260,7 @@
>
> // Adjust the VRegPHIUseCount map to account for the removal of
> this PHI node.
> for (unsigned i = 1; i != MPhi->getNumOperands(); i += 2)
> - --VRegPHIUseCount[BBVRegPair(MPhi->getOperand(i + 1).getMBB(),
> + --VRegPHIUseCount[BBVRegPair(MPhi->getOperand(i+1).getMBB()-
> >getNumber(),
> MPhi->getOperand(i).getReg())];
>
> // Now loop over all of the incoming arguments, changing them to
> copy into the
> @@ -266,7 +298,8 @@
> FindCopyInsertPoint(opBlock, MBB, SrcReg);
>
> // Insert the copy.
> - TII->copyRegToReg(opBlock, InsertPos, IncomingReg, SrcReg, RC,
> RC);
> + if (!reusedIncoming && IncomingReg)
> + TII->copyRegToReg(opBlock, InsertPos, IncomingReg, SrcReg,
> RC, RC);
>
> // Now update live variable information if we have it.
> Otherwise we're done
> if (!LV) continue;
> @@ -283,7 +316,7 @@
> // point later.
>
> // Is it used by any PHI instructions in this block?
> - bool ValueIsUsed = VRegPHIUseCount[BBVRegPair(&opBlock,
> SrcReg)] != 0;
> + bool ValueIsUsed =
> VRegPHIUseCount[BBVRegPair(opBlock.getNumber(), SrcReg)];
>
> // Okay, if we now know that the value is not live out of the
> block, we can
> // add a kill marker in this block saying that it kills the
> incoming value!
> @@ -293,11 +326,10 @@
> // terminator instruction at the end of the block may also use
> the value.
> // In this case, we should mark *it* as being the killing
> block, not the
> // copy.
> - MachineBasicBlock::iterator KillInst = prior(InsertPos);
> + MachineBasicBlock::iterator KillInst;
> MachineBasicBlock::iterator Term = opBlock.getFirstTerminator();
> - if (Term != opBlock.end()) {
> - if (Term->readsRegister(SrcReg))
> - KillInst = Term;
> + if (Term != opBlock.end() && Term->readsRegister(SrcReg)) {
> + KillInst = Term;
>
> // Check that no other terminators use values.
> #ifndef NDEBUG
> @@ -308,7 +340,17 @@
> "they are the first terminator in a block!");
> }
> #endif
> + } else if (reusedIncoming || !IncomingReg) {
> + // We may have to rewind a bit if we didn't insert a copy
> this time.
> + KillInst = Term;
> + while (KillInst != opBlock.begin())
> + if ((--KillInst)->readsRegister(SrcReg))
> + break;
> + } else {
> + // We just inserted this copy.
> + KillInst = prior(InsertPos);
> }
> + assert(KillInst->readsRegister(SrcReg) && "Cannot find kill
> instruction");
>
> // Finally, mark it killed.
> LV->addVirtualRegisterKilled(SrcReg, KillInst);
> @@ -319,9 +361,9 @@
> }
> }
>
> - // Really delete the PHI instruction now!
> - MF.DeleteMachineInstr(MPhi);
> - ++NumAtomic;
> + // Really delete the PHI instruction now, if it is not in the
> LoweredPHIs map.
> + if (reusedIncoming || !IncomingReg)
> + MF.DeleteMachineInstr(MPhi);
> }
>
> /// analyzePHINodes - Gather information about the PHI nodes in
> here. In
> @@ -335,7 +377,7 @@
> for (MachineBasicBlock::const_iterator BBI = I->begin(), BBE = I-
> >end();
> BBI != BBE && BBI->getOpcode() == TargetInstrInfo::PHI; +
> +BBI)
> for (unsigned i = 1, e = BBI->getNumOperands(); i != e; i += 2)
> - ++VRegPHIUseCount[BBVRegPair(BBI->getOperand(i + 1).getMBB(),
> + ++VRegPHIUseCount[BBVRegPair(BBI->getOperand(i+1).getMBB()-
> >getNumber(),
> BBI->getOperand(i).getReg())];
> }
>
> @@ -408,3 +450,34 @@
>
> return NMBB;
> }
> +
> +unsigned
> +PHIElimination::PHINodeTraits::getHashValue(const MachineInstr *MI) {
> + if (!MI || MI==getEmptyKey() || MI==getTombstoneKey())
> + return DenseMapInfo<MachineInstr*>::getHashValue(MI);
> + unsigned hash = 0;
> + for (unsigned ni = 1, ne = MI->getNumOperands(); ni != ne; ni += 2)
> + hash = hash*37 + DenseMapInfo<BBVRegPair>::
> + getHashValue(BBVRegPair(MI->getOperand(ni+1).getMBB()-
> >getNumber(),
> + MI->getOperand(ni).getReg()));
> + return hash;
> +}
> +
> +bool PHIElimination::PHINodeTraits::isEqual(const MachineInstr *LHS,
> + const MachineInstr
> *RHS) {
> + const MachineInstr *EmptyKey = getEmptyKey();
> + const MachineInstr *TombstoneKey = getTombstoneKey();
> + if (!LHS || !RHS || LHS==EmptyKey || RHS==EmptyKey ||
> + LHS==TombstoneKey || RHS==TombstoneKey)
> + return LHS==RHS;
> +
> + unsigned ne = LHS->getNumOperands();
> + if (ne != RHS->getNumOperands())
> + return false;
> + // Ignore operand 0, the defined register.
> + for (unsigned ni = 1; ni != ne; ni += 2)
> + if (LHS->getOperand(ni).getReg() != RHS-
> >getOperand(ni).getReg() ||
> + LHS->getOperand(ni+1).getMBB() != RHS->getOperand(ni
> +1).getMBB())
> + return false;
> + return true;
> +}
>
> Modified: llvm/trunk/lib/CodeGen/PHIElimination.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PHIElimination.h?rev=91549&r1=91548&r2=91549&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/CodeGen/PHIElimination.h (original)
> +++ llvm/trunk/lib/CodeGen/PHIElimination.h Wed Dec 16 12:55:53 2009
> @@ -16,8 +16,6 @@
> #include "llvm/CodeGen/MachineFunctionPass.h"
> #include "llvm/Target/TargetInstrInfo.h"
>
> -#include <map>
> -
> namespace llvm {
>
> /// Lower PHI instructions to copies.
> @@ -120,8 +118,8 @@
> return I;
> }
>
> - typedef std::pair<const MachineBasicBlock*, unsigned> BBVRegPair;
> - typedef std::map<BBVRegPair, unsigned> VRegPHIUse;
> + typedef std::pair<unsigned, unsigned> BBVRegPair;
> + typedef DenseMap<BBVRegPair, unsigned> VRegPHIUse;
>
> VRegPHIUse VRegPHIUseCount;
> PHIDefMap PHIDefs;
> @@ -129,6 +127,17 @@
>
> // Defs of PHI sources which are implicit_def.
> SmallPtrSet<MachineInstr*, 4> ImpDefs;
> +
> + // Lowered PHI nodes may be reused. We provide special DenseMap
> traits to
> + // match PHI nodes with identical arguments.
> + struct PHINodeTraits : public DenseMapInfo<MachineInstr*> {
> + static unsigned getHashValue(const MachineInstr *PtrVal);
> + static bool isEqual(const MachineInstr *LHS, const
> MachineInstr *RHS);
> + };
> +
> + // Map reusable lowered PHI node -> incoming join register.
> + typedef DenseMap<MachineInstr*, unsigned, PHINodeTraits>
> LoweredPHIMap;
> + LoweredPHIMap LoweredPHIs;
> };
>
> }
>
>
> _______________________________________________
> 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