[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