[llvm-commits] [llvm] r151111 - /llvm/trunk/lib/CodeGen/RegAllocFast.cpp
Chad Rosier
mcrosier at apple.com
Tue Feb 21 17:18:58 PST 2012
On Feb 21, 2012, at 5:02 PM, Jakob Stoklund Olesen wrote:
> Author: stoklund
> Date: Tue Feb 21 19:02:37 2012
> New Revision: 151111
>
> URL: http://llvm.org/viewvc/llvm-project?rev=151111&view=rev
> Log:
> Use SparseSet for the RAFast live virtual register map.
>
> This makes RAFast 4% faster, and it gets rid of the dodgy DenseMap
> iteration.
Very nice!! What exactly were you testing to get the 4% estimate?
Chad
> This also revealed that RAFast would sometimes dereference DenseMap
> iterators after erasing other elements from the map. That does seem to
> work in the current DenseMap implementation, but SparseSet doesn't allow
> it.
>
> Modified:
> llvm/trunk/lib/CodeGen/RegAllocFast.cpp
>
> Modified: llvm/trunk/lib/CodeGen/RegAllocFast.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocFast.cpp?rev=151111&r1=151110&r2=151111&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/RegAllocFast.cpp (original)
> +++ llvm/trunk/lib/CodeGen/RegAllocFast.cpp Tue Feb 21 19:02:37 2012
> @@ -32,6 +32,7 @@
> #include "llvm/ADT/IndexedMap.h"
> #include "llvm/ADT/SmallSet.h"
> #include "llvm/ADT/SmallVector.h"
> +#include "llvm/ADT/SparseSet.h"
> #include "llvm/ADT/Statistic.h"
> #include "llvm/ADT/STLExtras.h"
> #include <algorithm>
> @@ -68,16 +69,20 @@
> // Everything we know about a live virtual register.
> struct LiveReg {
> MachineInstr *LastUse; // Last instr to use reg.
> + unsigned VirtReg; // Virtual register number.
> unsigned PhysReg; // Currently held here.
> unsigned short LastOpNum; // OpNum on LastUse.
> bool Dirty; // Register needs spill.
>
> - LiveReg(unsigned p=0) : LastUse(0), PhysReg(p), LastOpNum(0),
> - Dirty(false) {}
> + explicit LiveReg(unsigned v)
> + : LastUse(0), VirtReg(v), PhysReg(0), LastOpNum(0), Dirty(false) {}
> +
> + unsigned getSparseSetKey() const {
> + return TargetRegisterInfo::virtReg2Index(VirtReg);
> + }
> };
>
> - typedef DenseMap<unsigned, LiveReg> LiveRegMap;
> - typedef LiveRegMap::value_type LiveRegEntry;
> + typedef SparseSet<LiveReg> LiveRegMap;
>
> // LiveVirtRegs - This map contains entries for each virtual register
> // that is currently available in a physical register.
> @@ -154,8 +159,16 @@
> void usePhysReg(MachineOperand&);
> void definePhysReg(MachineInstr *MI, unsigned PhysReg, RegState NewState);
> unsigned calcSpillCost(unsigned PhysReg) const;
> - void assignVirtToPhysReg(LiveRegEntry &LRE, unsigned PhysReg);
> - void allocVirtReg(MachineInstr *MI, LiveRegEntry &LRE, unsigned Hint);
> + void assignVirtToPhysReg(LiveReg&, unsigned PhysReg);
> + LiveRegMap::iterator findLiveVirtReg(unsigned VirtReg) {
> + return LiveVirtRegs.find(TargetRegisterInfo::virtReg2Index(VirtReg));
> + }
> + LiveRegMap::const_iterator findLiveVirtReg(unsigned VirtReg) const {
> + return LiveVirtRegs.find(TargetRegisterInfo::virtReg2Index(VirtReg));
> + }
> + LiveRegMap::iterator assignVirtToPhysReg(unsigned VReg, unsigned PhysReg);
> + LiveRegMap::iterator allocVirtReg(MachineInstr *MI, LiveRegMap::iterator,
> + unsigned Hint);
> LiveRegMap::iterator defineVirtReg(MachineInstr *MI, unsigned OpNum,
> unsigned VirtReg, unsigned Hint);
> LiveRegMap::iterator reloadVirtReg(MachineInstr *MI, unsigned OpNum,
> @@ -218,10 +231,9 @@
>
> /// killVirtReg - Mark virtreg as no longer available.
> void RAFast::killVirtReg(LiveRegMap::iterator LRI) {
> - addKillFlag(LRI->second);
> - const LiveReg &LR = LRI->second;
> - assert(PhysRegState[LR.PhysReg] == LRI->first && "Broken RegState mapping");
> - PhysRegState[LR.PhysReg] = regFree;
> + addKillFlag(*LRI);
> + assert(PhysRegState[LRI->PhysReg] == LRI->VirtReg && "Broken RegState mapping");
> + PhysRegState[LRI->PhysReg] = regFree;
> // Erase from LiveVirtRegs unless we're spilling in bulk.
> if (!isBulkSpilling)
> LiveVirtRegs.erase(LRI);
> @@ -231,7 +243,7 @@
> void RAFast::killVirtReg(unsigned VirtReg) {
> assert(TargetRegisterInfo::isVirtualRegister(VirtReg) &&
> "killVirtReg needs a virtual register");
> - LiveRegMap::iterator LRI = LiveVirtRegs.find(VirtReg);
> + LiveRegMap::iterator LRI = findLiveVirtReg(VirtReg);
> if (LRI != LiveVirtRegs.end())
> killVirtReg(LRI);
> }
> @@ -241,7 +253,7 @@
> void RAFast::spillVirtReg(MachineBasicBlock::iterator MI, unsigned VirtReg) {
> assert(TargetRegisterInfo::isVirtualRegister(VirtReg) &&
> "Spilling a physical register is illegal!");
> - LiveRegMap::iterator LRI = LiveVirtRegs.find(VirtReg);
> + LiveRegMap::iterator LRI = findLiveVirtReg(VirtReg);
> assert(LRI != LiveVirtRegs.end() && "Spilling unmapped virtual register");
> spillVirtReg(MI, LRI);
> }
> @@ -249,18 +261,18 @@
> /// spillVirtReg - Do the actual work of spilling.
> void RAFast::spillVirtReg(MachineBasicBlock::iterator MI,
> LiveRegMap::iterator LRI) {
> - LiveReg &LR = LRI->second;
> - assert(PhysRegState[LR.PhysReg] == LRI->first && "Broken RegState mapping");
> + LiveReg &LR = *LRI;
> + assert(PhysRegState[LR.PhysReg] == LRI->VirtReg && "Broken RegState mapping");
>
> if (LR.Dirty) {
> // If this physreg is used by the instruction, we want to kill it on the
> // instruction, not on the spill.
> bool SpillKill = LR.LastUse != MI;
> LR.Dirty = false;
> - DEBUG(dbgs() << "Spilling " << PrintReg(LRI->first, TRI)
> + DEBUG(dbgs() << "Spilling " << PrintReg(LRI->VirtReg, TRI)
> << " in " << PrintReg(LR.PhysReg, TRI));
> - const TargetRegisterClass *RC = MRI->getRegClass(LRI->first);
> - int FI = getStackSpaceFor(LRI->first, RC);
> + const TargetRegisterClass *RC = MRI->getRegClass(LRI->VirtReg);
> + int FI = getStackSpaceFor(LRI->VirtReg, RC);
> DEBUG(dbgs() << " to stack slot #" << FI << "\n");
> TII->storeRegToStackSlot(*MBB, MI, LR.PhysReg, SpillKill, FI, RC, TRI);
> ++NumStores; // Update statistics
> @@ -268,7 +280,8 @@
> // If this register is used by DBG_VALUE then insert new DBG_VALUE to
> // identify spilled location as the place to find corresponding variable's
> // value.
> - SmallVector<MachineInstr *, 4> &LRIDbgValues = LiveDbgValueMap[LRI->first];
> + SmallVector<MachineInstr *, 4> &LRIDbgValues =
> + LiveDbgValueMap[LRI->VirtReg];
> for (unsigned li = 0, le = LRIDbgValues.size(); li != le; ++li) {
> MachineInstr *DBG = LRIDbgValues[li];
> const MDNode *MDPtr =
> @@ -431,8 +444,11 @@
> DEBUG(dbgs() << PrintReg(VirtReg, TRI) << " corresponding "
> << PrintReg(PhysReg, TRI) << " is reserved already.\n");
> return spillImpossible;
> - default:
> - return LiveVirtRegs.lookup(VirtReg).Dirty ? spillDirty : spillClean;
> + default: {
> + LiveRegMap::const_iterator I = findLiveVirtReg(VirtReg);
> + assert(I != LiveVirtRegs.end() && "Missing VirtReg entry");
> + return I->Dirty ? spillDirty : spillClean;
> + }
> }
>
> // This is a disabled register, add up cost of aliases.
> @@ -450,10 +466,13 @@
> break;
> case regReserved:
> return spillImpossible;
> - default:
> - Cost += LiveVirtRegs.lookup(VirtReg).Dirty ? spillDirty : spillClean;
> + default: {
> + LiveRegMap::const_iterator I = findLiveVirtReg(VirtReg);
> + assert(I != LiveVirtRegs.end() && "Missing VirtReg entry");
> + Cost += I->Dirty ? spillDirty : spillClean;
> break;
> }
> + }
> }
> return Cost;
> }
> @@ -463,17 +482,27 @@
> /// that PhysReg is the proper container for VirtReg now. The physical
> /// register must not be used for anything else when this is called.
> ///
> -void RAFast::assignVirtToPhysReg(LiveRegEntry &LRE, unsigned PhysReg) {
> - DEBUG(dbgs() << "Assigning " << PrintReg(LRE.first, TRI) << " to "
> +void RAFast::assignVirtToPhysReg(LiveReg &LR, unsigned PhysReg) {
> + DEBUG(dbgs() << "Assigning " << PrintReg(LR.VirtReg, TRI) << " to "
> << PrintReg(PhysReg, TRI) << "\n");
> - PhysRegState[PhysReg] = LRE.first;
> - assert(!LRE.second.PhysReg && "Already assigned a physreg");
> - LRE.second.PhysReg = PhysReg;
> + PhysRegState[PhysReg] = LR.VirtReg;
> + assert(!LR.PhysReg && "Already assigned a physreg");
> + LR.PhysReg = PhysReg;
> +}
> +
> +RAFast::LiveRegMap::iterator
> +RAFast::assignVirtToPhysReg(unsigned VirtReg, unsigned PhysReg) {
> + LiveRegMap::iterator LRI = findLiveVirtReg(VirtReg);
> + assert(LRI != LiveVirtRegs.end() && "VirtReg disappeared");
> + assignVirtToPhysReg(*LRI, PhysReg);
> + return LRI;
> }
>
> /// allocVirtReg - Allocate a physical register for VirtReg.
> -void RAFast::allocVirtReg(MachineInstr *MI, LiveRegEntry &LRE, unsigned Hint) {
> - const unsigned VirtReg = LRE.first;
> +RAFast::LiveRegMap::iterator RAFast::allocVirtReg(MachineInstr *MI,
> + LiveRegMap::iterator LRI,
> + unsigned Hint) {
> + const unsigned VirtReg = LRI->VirtReg;
>
> assert(TargetRegisterInfo::isVirtualRegister(VirtReg) &&
> "Can only allocate virtual registers");
> @@ -492,7 +521,9 @@
> if (Cost < spillDirty) {
> if (Cost)
> definePhysReg(MI, Hint, regFree);
> - return assignVirtToPhysReg(LRE, Hint);
> + // definePhysReg may kill virtual registers and modify LiveVirtRegs.
> + // That invalidates LRI, so run a new lookup for VirtReg.
> + return assignVirtToPhysReg(VirtReg, Hint);
> }
> }
>
> @@ -501,8 +532,10 @@
> // First try to find a completely free register.
> for (ArrayRef<unsigned>::iterator I = AO.begin(), E = AO.end(); I != E; ++I) {
> unsigned PhysReg = *I;
> - if (PhysRegState[PhysReg] == regFree && !UsedInInstr.test(PhysReg))
> - return assignVirtToPhysReg(LRE, PhysReg);
> + if (PhysRegState[PhysReg] == regFree && !UsedInInstr.test(PhysReg)) {
> + assignVirtToPhysReg(*LRI, PhysReg);
> + return LRI;
> + }
> }
>
> DEBUG(dbgs() << "Allocating " << PrintReg(VirtReg) << " from "
> @@ -515,21 +548,25 @@
> DEBUG(dbgs() << "\tCost: " << Cost << "\n");
> DEBUG(dbgs() << "\tBestCost: " << BestCost << "\n");
> // Cost is 0 when all aliases are already disabled.
> - if (Cost == 0)
> - return assignVirtToPhysReg(LRE, *I);
> + if (Cost == 0) {
> + assignVirtToPhysReg(*LRI, *I);
> + return LRI;
> + }
> if (Cost < BestCost)
> BestReg = *I, BestCost = Cost;
> }
>
> if (BestReg) {
> definePhysReg(MI, BestReg, regFree);
> - return assignVirtToPhysReg(LRE, BestReg);
> + // definePhysReg may kill virtual registers and modify LiveVirtRegs.
> + // That invalidates LRI, so run a new lookup for VirtReg.
> + return assignVirtToPhysReg(VirtReg, BestReg);
> }
>
> // Nothing we can do. Report an error and keep going with a bad allocation.
> MI->emitError("ran out of registers during register allocation");
> definePhysReg(MI, *AO.begin(), regFree);
> - assignVirtToPhysReg(LRE, *AO.begin());
> + return assignVirtToPhysReg(VirtReg, *AO.begin());
> }
>
> /// defineVirtReg - Allocate a register for VirtReg and mark it as dirty.
> @@ -540,8 +577,7 @@
> "Not a virtual register");
> LiveRegMap::iterator LRI;
> bool New;
> - tie(LRI, New) = LiveVirtRegs.insert(std::make_pair(VirtReg, LiveReg()));
> - LiveReg &LR = LRI->second;
> + tie(LRI, New) = LiveVirtRegs.insert(LiveReg(VirtReg));
> if (New) {
> // If there is no hint, peek at the only use of this register.
> if ((!Hint || !TargetRegisterInfo::isPhysicalRegister(Hint)) &&
> @@ -551,18 +587,18 @@
> if (UseMI.isCopyLike())
> Hint = UseMI.getOperand(0).getReg();
> }
> - allocVirtReg(MI, *LRI, Hint);
> - } else if (LR.LastUse) {
> + LRI = allocVirtReg(MI, LRI, Hint);
> + } else if (LRI->LastUse) {
> // Redefining a live register - kill at the last use, unless it is this
> // instruction defining VirtReg multiple times.
> - if (LR.LastUse != MI || LR.LastUse->getOperand(LR.LastOpNum).isUse())
> - addKillFlag(LR);
> + if (LRI->LastUse != MI || LRI->LastUse->getOperand(LRI->LastOpNum).isUse())
> + addKillFlag(*LRI);
> }
> - assert(LR.PhysReg && "Register not assigned");
> - LR.LastUse = MI;
> - LR.LastOpNum = OpNum;
> - LR.Dirty = true;
> - UsedInInstr.set(LR.PhysReg);
> + assert(LRI->PhysReg && "Register not assigned");
> + LRI->LastUse = MI;
> + LRI->LastOpNum = OpNum;
> + LRI->Dirty = true;
> + UsedInInstr.set(LRI->PhysReg);
> return LRI;
> }
>
> @@ -574,18 +610,17 @@
> "Not a virtual register");
> LiveRegMap::iterator LRI;
> bool New;
> - tie(LRI, New) = LiveVirtRegs.insert(std::make_pair(VirtReg, LiveReg()));
> - LiveReg &LR = LRI->second;
> + tie(LRI, New) = LiveVirtRegs.insert(LiveReg(VirtReg));
> MachineOperand &MO = MI->getOperand(OpNum);
> if (New) {
> - allocVirtReg(MI, *LRI, Hint);
> + LRI = allocVirtReg(MI, LRI, Hint);
> const TargetRegisterClass *RC = MRI->getRegClass(VirtReg);
> int FrameIndex = getStackSpaceFor(VirtReg, RC);
> DEBUG(dbgs() << "Reloading " << PrintReg(VirtReg, TRI) << " into "
> - << PrintReg(LR.PhysReg, TRI) << "\n");
> - TII->loadRegFromStackSlot(*MBB, MI, LR.PhysReg, FrameIndex, RC, TRI);
> + << PrintReg(LRI->PhysReg, TRI) << "\n");
> + TII->loadRegFromStackSlot(*MBB, MI, LRI->PhysReg, FrameIndex, RC, TRI);
> ++NumLoads;
> - } else if (LR.Dirty) {
> + } else if (LRI->Dirty) {
> if (isLastUseOfLocalReg(MO)) {
> DEBUG(dbgs() << "Killing last use: " << MO << "\n");
> if (MO.isUse())
> @@ -610,10 +645,10 @@
> DEBUG(dbgs() << "Clearing clean dead: " << MO << "\n");
> MO.setIsDead(false);
> }
> - assert(LR.PhysReg && "Register not assigned");
> - LR.LastUse = MI;
> - LR.LastOpNum = OpNum;
> - UsedInInstr.set(LR.PhysReg);
> + assert(LRI->PhysReg && "Register not assigned");
> + LRI->LastUse = MI;
> + LRI->LastOpNum = OpNum;
> + UsedInInstr.set(LRI->PhysReg);
> return LRI;
> }
>
> @@ -690,7 +725,7 @@
> DEBUG(dbgs() << "Operand " << i << "("<< MO << ") is tied to operand "
> << DefIdx << ".\n");
> LiveRegMap::iterator LRI = reloadVirtReg(MI, i, Reg, 0);
> - unsigned PhysReg = LRI->second.PhysReg;
> + unsigned PhysReg = LRI->PhysReg;
> setPhysReg(MI, i, PhysReg);
> // Note: we don't update the def operand yet. That would cause the normal
> // def-scan to attempt spilling.
> @@ -699,7 +734,7 @@
> // Reload the register, but don't assign to the operand just yet.
> // That would confuse the later phys-def processing pass.
> LiveRegMap::iterator LRI = reloadVirtReg(MI, i, Reg, 0);
> - PartialDefs.push_back(LRI->second.PhysReg);
> + PartialDefs.push_back(LRI->PhysReg);
> }
> }
>
> @@ -713,7 +748,7 @@
> continue;
> // Note: defineVirtReg may invalidate MO.
> LiveRegMap::iterator LRI = defineVirtReg(MI, i, Reg, 0);
> - unsigned PhysReg = LRI->second.PhysReg;
> + unsigned PhysReg = LRI->PhysReg;
> if (setPhysReg(MI, i, PhysReg))
> VirtDead.push_back(Reg);
> }
> @@ -794,7 +829,7 @@
> DEBUG(dbgs() << "\nAllocating " << *MBB);
>
> PhysRegState.assign(TRI->getNumRegs(), regDisabled);
> - assert(LiveVirtRegs.empty() && "Mapping not cleared form last block?");
> + assert(LiveVirtRegs.empty() && "Mapping not cleared from last block?");
>
> MachineBasicBlock::iterator MII = MBB->begin();
>
> @@ -822,25 +857,26 @@
> case regReserved:
> dbgs() << "*";
> break;
> - default:
> + default: {
> dbgs() << '=' << PrintReg(PhysRegState[Reg]);
> - if (LiveVirtRegs[PhysRegState[Reg]].Dirty)
> + LiveRegMap::iterator I = findLiveVirtReg(PhysRegState[Reg]);
> + assert(I != LiveVirtRegs.end() && "Missing VirtReg entry");
> + if (I->Dirty)
> dbgs() << "*";
> - assert(LiveVirtRegs[PhysRegState[Reg]].PhysReg == Reg &&
> - "Bad inverse map");
> + assert(I->PhysReg == Reg && "Bad inverse map");
> break;
> }
> + }
> }
> dbgs() << '\n';
> // Check that LiveVirtRegs is the inverse.
> for (LiveRegMap::iterator i = LiveVirtRegs.begin(),
> e = LiveVirtRegs.end(); i != e; ++i) {
> - assert(TargetRegisterInfo::isVirtualRegister(i->first) &&
> + assert(TargetRegisterInfo::isVirtualRegister(i->VirtReg) &&
> "Bad map key");
> - assert(TargetRegisterInfo::isPhysicalRegister(i->second.PhysReg) &&
> + assert(TargetRegisterInfo::isPhysicalRegister(i->PhysReg) &&
> "Bad map value");
> - assert(PhysRegState[i->second.PhysReg] == i->first &&
> - "Bad inverse map");
> + assert(PhysRegState[i->PhysReg] == i->VirtReg && "Bad inverse map");
> }
> });
>
> @@ -854,9 +890,9 @@
> if (!MO.isReg()) continue;
> unsigned Reg = MO.getReg();
> if (!TargetRegisterInfo::isVirtualRegister(Reg)) continue;
> - LiveRegMap::iterator LRI = LiveVirtRegs.find(Reg);
> + LiveRegMap::iterator LRI = findLiveVirtReg(Reg);
> if (LRI != LiveVirtRegs.end())
> - setPhysReg(MI, i, LRI->second.PhysReg);
> + setPhysReg(MI, i, LRI->PhysReg);
> else {
> int SS = StackSlotForVirtReg[Reg];
> if (SS == -1) {
> @@ -971,7 +1007,7 @@
> if (!TargetRegisterInfo::isVirtualRegister(Reg)) continue;
> if (MO.isUse()) {
> LiveRegMap::iterator LRI = reloadVirtReg(MI, i, Reg, CopyDst);
> - unsigned PhysReg = LRI->second.PhysReg;
> + unsigned PhysReg = LRI->PhysReg;
> CopySrc = (CopySrc == Reg || CopySrc == PhysReg) ? PhysReg : 0;
> if (setPhysReg(MI, i, PhysReg))
> killVirtReg(LRI);
> @@ -1027,7 +1063,7 @@
> continue;
> }
> LiveRegMap::iterator LRI = defineVirtReg(MI, i, Reg, CopySrc);
> - unsigned PhysReg = LRI->second.PhysReg;
> + unsigned PhysReg = LRI->PhysReg;
> if (setPhysReg(MI, i, PhysReg)) {
> VirtDead.push_back(Reg);
> CopyDst = 0; // cancel coalescing;
> @@ -1089,6 +1125,7 @@
> // initialize the virtual->physical register map to have a 'null'
> // mapping for all virtual registers
> StackSlotForVirtReg.resize(MRI->getNumVirtRegs());
> + LiveVirtRegs.setUniverse(MRI->getNumVirtRegs());
>
> // Loop over all of the basic blocks, eliminating virtual register references
> for (MachineFunction::iterator MBBi = Fn.begin(), MBBe = Fn.end();
>
>
> _______________________________________________
> 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