[PATCH] Modify the register scavenger to operate on reg units when checking liveness

Chad Rosier mcrosier at codeaurora.org
Thu Jul 31 14:29:57 PDT 2014


> Hello All,
>
> This patch modifies the register scavenger to operate on reg units
> (instead of full registers) when tracking liveness of registers. The code
> is more general and should perform better than the old version.
>
> Here is the patch in file and text format.
>
> Thanks
>
> Pedro
>
>
>
> Index: include/llvm/CodeGen/MachineRegisterInfo.h
> ===================================================================
> --- include/llvm/CodeGen/MachineRegisterInfo.h	(revision 214407)
> +++ include/llvm/CodeGen/MachineRegisterInfo.h	(working copy)
> @@ -516,6 +516,23 @@
>    /// That function will return NULL if the virtual registers have
> incompatible
>    /// constraints.
>    void replaceRegWith(unsigned FromReg, unsigned ToReg);
> +
> +  /// replaceVRegWithPhysReg - Replace all instances of virtual register
> +  /// fromReg with physical register ToReg, also applies sub-registers in
> +  /// operands to map to final physical register.
> +  ///
> +  /// Example:
> +  ///
> +  /// vreg10 =
> +  /// = vreg10.sub0
> +  /// = vreg10.sub1
> +  ///
> +  /// ==>
> +  ///
> +  /// R5_R6 =
> +  /// = R5
> +  /// = R6
> +  void replaceVRegWithPhysReg(unsigned FromReg, unsigned ToReg);
>
>    /// getVRegDef - Return the machine instr that defines the specified
> virtual
>    /// register or null if none is found.  This assumes that the code is
> in SSA
> Index: include/llvm/CodeGen/RegisterScavenging.h
> ===================================================================
> --- include/llvm/CodeGen/RegisterScavenging.h	(revision 214407)
> +++ include/llvm/CodeGen/RegisterScavenging.h	(working copy)
> @@ -35,6 +35,7 @@
>    MachineBasicBlock *MBB;
>    MachineBasicBlock::iterator MBBI;
>    unsigned NumPhysRegs;
> +  unsigned NumRegUnits;
>
>    /// Tracking - True if RegScavenger is currently tracking the liveness
> of
>    /// registers.
> @@ -62,18 +63,20 @@
>    ///
>    BitVector CalleeSavedRegs;
>
> -  /// RegsAvailable - The current state of all the physical registers
> immediately
> -  /// before MBBI. One bit per physical register. If bit is set that
> means it's
> -  /// available, unset means the register is currently being used.
> -  BitVector RegsAvailable;
> +  /// RegUnitsAvailable - The current state of each reg unit immediatelly
> +  /// before MBBI. One bit per register unit. If bit is not set it means
> any
> +  /// register containing that register unit is currently being used.
> +  BitVector RegUnitsAvailable;
>
>    // These BitVectors are only used internally to forward(). They are
> members
>    // to avoid frequent reallocations.
> -  BitVector KillRegs, DefRegs;
> +  BitVector KillRegUnits, DefRegUnits;
> +  BitVector TmpRegUnits;
> +  BitVector TmpRegs;
>
>  public:
>    RegScavenger()
> -    : MBB(nullptr), NumPhysRegs(0), Tracking(false) {}
> +    : MBB(nullptr), NumPhysRegs(0), NumRegUnits(0), Tracking(false) {}
>
>    /// enterBasicBlock - Start tracking liveness from the begin of the
> specific
>    /// basic block.
> @@ -164,33 +167,35 @@
>    /// isReserved - Returns true if a register is reserved. It is never
> "unused".
>    bool isReserved(unsigned Reg) const { return MRI->isReserved(Reg); }
>
> -  /// isUsed - Test if a register is currently being used.  When called
> by the
> -  /// isAliasUsed function, we only check isReserved if this is the
> original
> -  /// register, not an alias register.
> -  ///
> -  bool isUsed(unsigned Reg, bool CheckReserved = true) const   {
> -    return !RegsAvailable.test(Reg) || (CheckReserved &&
> isReserved(Reg));
> -  }
> +  /// isUsed - Test if a register is currently being used. Check reserved
> +  /// registers also if requested.
> +  bool isUsed(unsigned Reg, bool CheckReserved = true) const;
>
> -  /// isAliasUsed - Is Reg or an alias currently in use?
> -  bool isAliasUsed(unsigned Reg) const;
> -
> -  /// setUsed / setUnused - Mark the state of one or a number of
> registers.
> +  /// setUsed / setUnused - Mark the state of one or a number of register
> units.
>    ///
>    void setUsed(BitVector &Regs) {
> -    RegsAvailable.reset(Regs);
> +    RegUnitsAvailable.reset(Regs);
>    }
>    void setUnused(BitVector &Regs) {
> -    RegsAvailable |= Regs;
> +    RegUnitsAvailable |= Regs;
>    }
>
> -  /// Processes the current instruction and fill the KillRegs and DefRegs
> bit
> -  /// vectors.
> +  /// Processes the current instruction and fill the KillRegUnits and
> +  /// DefRegUnits bit vectors.
>    void determineKillsAndDefs();
> +
> +  /// Add all Reg Units that Reg contains to BV.
> +  void addRegUnits(BitVector &BV, unsigned Reg);
> +
> +  /// Remove all Reg Units that Reg contains from BV.
> +  void subRegUnits(BitVector &BV, unsigned Reg);
> +
> +  /// Add all Reg Units that Regs in RBV contain to BV.
> +  void addRegUnits(BitVector &BV, BitVector &RBV);
>
> -  /// Add Reg and all its sub-registers to BV.
> -  void addRegWithSubRegs(BitVector &BV, unsigned Reg);
> -
> +  /// Remove all Reg Units that Regs in RBV contain from BV.
> +  void subRegUnits(BitVector &BV, BitVector &RBV);
> +
>    /// findSurvivorReg - Return the candidate register that is unused for
> the
>    /// longest after StartMI. UseMI is set to the instruction where the
> search
>    /// stopped.
> Index: lib/CodeGen/MachineRegisterInfo.cpp
> ===================================================================
> --- lib/CodeGen/MachineRegisterInfo.cpp	(revision 214407)
> +++ lib/CodeGen/MachineRegisterInfo.cpp	(working copy)
> @@ -294,7 +294,32 @@
>    }
>  }
>
> +/// replaceVRegWithPhysReg - Replace all instances of FromReg with ToReg
> in the
> +/// machine function. Note that ToReg must be a physical register; that
> is
> +/// important as we process the sub-registers applied to the operand to
> generate
> +/// a proper phys reg.
>
> +void MachineRegisterInfo::replaceVRegWithPhysReg(unsigned FromReg,
> +                                                 unsigned ToReg) {
> +
> +  const TargetRegisterInfo *TRI = getTargetRegisterInfo();
> +  // TODO: This could be more efficient by bulk changing the operands.
> +  for (reg_iterator I = reg_begin(FromReg), E = reg_end(); I != E; ) {
> +    MachineOperand &O = *I;
> +    ++I;
> +    if (O.getSubReg()) {
> +      unsigned SubReg = TRI->getSubReg(ToReg, O.getSubReg());
> +      assert(SubReg);
> +      setPhysRegUsed(SubReg);
> +      O.setReg(SubReg);
> +      O.setSubReg(0);
> +    } else {
> +      setPhysRegUsed(ToReg);
> +      O.setReg(ToReg);
> +    }
> +  }
> +}
> +
>  /// getVRegDef - Return the machine instr that defines the specified
> virtual
>  /// register or null if none is found.  This assumes that the code is in
> SSA
>  /// form, so there should only be one definition.
> Index: lib/CodeGen/PrologEpilogInserter.cpp
> ===================================================================
> --- lib/CodeGen/PrologEpilogInserter.cpp	(revision 214407)
> +++ lib/CodeGen/PrologEpilogInserter.cpp	(working copy)
> @@ -837,7 +837,8 @@
>  /// FIXME: Iterating over the instruction stream is unnecessary. We can
> simply
>  /// iterate over the vreg use list, which at this point only contains
> machine
>  /// operands for which eliminateFrameIndex need a new scratch reg.
> -void PEI::scavengeFrameVirtualRegs(MachineFunction &Fn) {
> +void
> +PEI::scavengeFrameVirtualRegs(MachineFunction &Fn) {
>    // Run through the instructions and find any virtual registers.
>    for (MachineFunction::iterator BB = Fn.begin(),
>         E = Fn.end(); BB != E; ++BB) {
> @@ -888,7 +889,7 @@
>            // Replace this reference to the virtual register with the
>            // scratch register.
>            assert (ScratchReg && "Missing scratch register!");
> -          Fn.getRegInfo().replaceRegWith(Reg, ScratchReg);
> +          Fn.getRegInfo().replaceVRegWithPhysReg(Reg, ScratchReg);
>
>            // Because this instruction was processed by the RS before this
>            // register was allocated, make sure that the RS now records
> the
> Index: lib/CodeGen/RegisterScavenging.cpp
> ===================================================================
> --- lib/CodeGen/RegisterScavenging.cpp	(revision 214407)
> +++ lib/CodeGen/RegisterScavenging.cpp	(working copy)
> @@ -30,20 +30,21 @@
>
>  #define DEBUG_TYPE "reg-scavenging"
>
> -/// setUsed - Set the register and its sub-registers as being used.
> -void RegScavenger::setUsed(unsigned Reg) {
> -  for (MCSubRegIterator SubRegs(Reg, TRI, /*IncludeSelf=*/true);
> -       SubRegs.isValid(); ++SubRegs)
> -    RegsAvailable.reset(*SubRegs);
> -}
> -
> -bool RegScavenger::isAliasUsed(unsigned Reg) const {
> -  for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI)
> -    if (isUsed(*AI, *AI == Reg))
> +bool RegScavenger::isUsed(unsigned Reg, bool CheckReserved) const   {
> +  if (CheckReserved && isReserved(Reg))
> +    return true;
> +  for (MCRegUnitIterator RUI(Reg, TRI); RUI.isValid(); ++RUI)
> +    if (!RegUnitsAvailable.test(*RUI))
>        return true;
>    return false;
>  }
>
> +/// setUsed - Set the register units of this register as used.
> +void RegScavenger::setUsed(unsigned Reg) {
> +  for (MCRegUnitIterator RUI(Reg, TRI); RUI.isValid(); ++RUI)
> +    RegUnitsAvailable.reset(*RUI);
> +}
> +
>  void RegScavenger::initRegState() {
>    for (SmallVectorImpl<ScavengedInfo>::iterator I = Scavenged.begin(),
>           IE = Scavenged.end(); I != IE; ++I) {
> @@ -51,8 +52,8 @@
>      I->Restore = nullptr;
>    }
>
> -  // All registers started out unused.
> -  RegsAvailable.set();
> +  // All register units start out unused.
> +  RegUnitsAvailable.set();
>
>    if (!MBB)
>      return;
> @@ -77,6 +78,9 @@
>
>    assert((NumPhysRegs == 0 || NumPhysRegs == TRI->getNumRegs()) &&
>           "Target changed?");
> +
> +  assert((NumRegUnits == 0 || NumRegUnits == TRI->getNumRegUnits()) &&
> +         "Target changed?");
>
>    // It is not possible to use the register scavenger after late
> optimization
>    // passes that don't preserve accurate liveness information.
> @@ -86,11 +90,15 @@
>    // Self-initialize.
>    if (!MBB) {
>      NumPhysRegs = TRI->getNumRegs();
> -    RegsAvailable.resize(NumPhysRegs);
> -    KillRegs.resize(NumPhysRegs);
> -    DefRegs.resize(NumPhysRegs);
> +    TmpRegs.resize(NumPhysRegs);
> +
> +    NumRegUnits = TRI->getNumRegUnits();
> +    RegUnitsAvailable.resize(NumRegUnits);
> +    KillRegUnits.resize(NumRegUnits);
> +    DefRegUnits.resize(NumRegUnits);
> +    TmpRegUnits.resize(NumRegUnits);
>
> -    // Create callee-saved registers bitvector.
> +    // Create callee-saved registers bitvector. (FIXME: Why is this
> needed?)

I believe the callee-saved register is an ancient artifact.

Jakob simplified the FindUnusedReg function here:
http://llvm.org/viewvc/llvm-project?view=revision&revision=79369

AFAICK, this should have been removed at that time.

 Chad

>      CalleeSavedRegs.resize(NumPhysRegs);
>      const MCPhysReg *CSRegs = TRI->getCalleeSavedRegs(&MF);
>      if (CSRegs != nullptr)
> @@ -104,12 +112,28 @@
>    Tracking = false;
>  }
>
> -void RegScavenger::addRegWithSubRegs(BitVector &BV, unsigned Reg) {
> -  for (MCSubRegIterator SubRegs(Reg, TRI, /*IncludeSelf=*/true);
> -       SubRegs.isValid(); ++SubRegs)
> -    BV.set(*SubRegs);
> +void RegScavenger::addRegUnits(BitVector &BV, unsigned Reg) {
> +  for (MCRegUnitIterator RUI(Reg, TRI); RUI.isValid(); ++RUI)
> +    BV.set(*RUI);
>  }
>
> +void RegScavenger::subRegUnits(BitVector &BV, unsigned Reg) {
> +  for (MCRegUnitIterator RUI(Reg, TRI); RUI.isValid(); ++RUI)
> +    BV.reset(*RUI);
> +}
> +
> +void RegScavenger::addRegUnits(BitVector &BV, BitVector &RBV) {
> +  for (int Reg = RBV.find_first(); Reg != -1; Reg = RBV.find_next(Reg))
> +    if (Reg)
> +      addRegUnits(BV, Reg);
> +}
> +
> +void RegScavenger::subRegUnits(BitVector &BV, BitVector &RBV) {
> +  for (int Reg = RBV.find_first(); Reg != -1; Reg = RBV.find_next(Reg))
> +    if (Reg)
> +      subRegUnits(BV, Reg);
> +}
> +
>  void RegScavenger::determineKillsAndDefs() {
>    assert(Tracking && "Must be tracking to determine kills and defs");
>
> @@ -122,12 +146,30 @@
>    // predicated, conservatively assume "kill" markers do not actually
> kill the
>    // register. Similarly ignores "dead" markers.
>    bool isPred = TII->isPredicated(MI);
> -  KillRegs.reset();
> -  DefRegs.reset();
> +  KillRegUnits.reset();
> +  DefRegUnits.reset();
>    for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
>      const MachineOperand &MO = MI->getOperand(i);
> -    if (MO.isRegMask())
> -      (isPred ? DefRegs : KillRegs).setBitsNotInMask(MO.getRegMask());
> +    if (MO.isRegMask()) {
> +
> +      //get the clobbered registers.
> +      TmpRegs.setBitsInMask(MO.getRegMask());
> +      TmpRegs.flip();
> +
> +      TmpRegUnits.reset();
> +
> +      // Add all the clobbered registers as reg units.
> +      addRegUnits(TmpRegUnits, TmpRegs);
> +
> +      // Get the preserved registers.
> +      TmpRegs.setBitsInMask(MO.getRegMask());
> +
> +      // Remove all the preserved reg units.
> +      subRegUnits(TmpRegUnits, TmpRegs);
> +
> +      // Apply the mask.
> +      (isPred ? DefRegUnits : KillRegUnits) |= TmpRegUnits;
> +    }
>      if (!MO.isReg())
>        continue;
>      unsigned Reg = MO.getReg();
> @@ -139,13 +181,13 @@
>        if (MO.isUndef())
>          continue;
>        if (!isPred && MO.isKill())
> -        addRegWithSubRegs(KillRegs, Reg);
> +        addRegUnits(KillRegUnits, Reg);
>      } else {
>        assert(MO.isDef());
>        if (!isPred && MO.isDead())
> -        addRegWithSubRegs(KillRegs, Reg);
> +        addRegUnits(KillRegUnits, Reg);
>        else
> -        addRegWithSubRegs(DefRegs, Reg);
> +        addRegUnits(DefRegUnits, Reg);
>      }
>    }
>  }
> @@ -158,8 +200,8 @@
>      determineKillsAndDefs();
>
>      // Commit the changes.
> -    setUsed(KillRegs);
> -    setUnused(DefRegs);
> +    setUsed(KillRegUnits);
> +    setUnused(DefRegUnits);
>    }
>
>    if (MBBI == MBB->begin()) {
> @@ -223,11 +265,19 @@
>              SubUsed = true;
>              break;
>            }
> -        if (!SubUsed) {
> +        bool SuperUsed = false;
> +        for (MCSuperRegIterator SR(Reg, TRI); SR.isValid(); ++SR) {
> +          if (isUsed(*SR)) {
> +            SuperUsed = true;
> +            break;
> +          }
> +        }
> +        if (!SubUsed && !SuperUsed) {
>            MBB->getParent()->verify(nullptr, "In Register Scavenger");
>            llvm_unreachable("Using an undefined register!");
>          }
>          (void)SubUsed;
> +        (void)SuperUsed;
>        }
>      } else {
>        assert(MO.isDef());
> @@ -243,13 +293,19 @@
>  #endif // NDEBUG
>
>    // Commit the changes.
> -  setUnused(KillRegs);
> -  setUsed(DefRegs);
> +  setUnused(KillRegUnits);
> +  setUsed(DefRegUnits);
>  }
>
>  void RegScavenger::getRegsUsed(BitVector &used, bool includeReserved) {
> -  used = RegsAvailable;
> -  used.flip();
> +  for (unsigned i = 1; i != NumPhysRegs; ++i) {
> +    for (MCRegUnitIterator RUI(i, TRI); RUI.isValid(); ++RUI) {
> +      if (!RegUnitsAvailable.test(*RUI)) {
> +        used.set(i);
> +        break;
> +      }
> +    }
> +  }
>    if (includeReserved)
>      used |= MRI->getReservedRegs();
>    else
> @@ -259,7 +315,7 @@
>  unsigned RegScavenger::FindUnusedReg(const TargetRegisterClass *RC) const
> {
>    for (TargetRegisterClass::iterator I = RC->begin(), E = RC->end();
>         I != E; ++I)
> -    if (!isAliasUsed(*I)) {
> +    if (!isUsed(*I)) {
>        DEBUG(dbgs() << "Scavenger found unused reg: " << TRI->getName(*I)
> <<
>              "\n");
>        return *I;
> @@ -273,13 +329,13 @@
>    BitVector Mask(TRI->getNumRegs());
>    for (TargetRegisterClass::iterator I = RC->begin(), E = RC->end();
>         I != E; ++I)
> -    if (!isAliasUsed(*I))
> +    if (!isUsed(*I))
>        Mask.set(*I);
>    return Mask;
>  }
>
>  /// findSurvivorReg - Return the candidate register that is unused for
> the
> -/// longest after StargMII. UseMI is set to the instruction where the
> search
> +/// longest after StartMII. UseMI is set to the instruction where the
> search
>  /// stopped.
>  ///
>  /// No more than InstrLimit instructions are inspected.
> @@ -375,9 +431,7 @@
>    }
>
>    // Try to find a register that's unused if there is one, as then we
> won't
> -  // have to spill. Search explicitly rather than masking out based on
> -  // RegsAvailable, as RegsAvailable does not take aliases into account.
> -  // That's what getRegsAvailable() is for.
> +  // have to spill.
>    BitVector Available = getRegsAvailable(RC);
>    Available &= Candidates;
>    if (Available.any())
> @@ -388,7 +442,7 @@
>    unsigned SReg = findSurvivorReg(I, Candidates, 25, UseMI);
>
>    // If we found an unused register there is no reason to spill it.
> -  if (!isAliasUsed(SReg)) {
> +  if (!isUsed(SReg)) {
>      DEBUG(dbgs() << "Scavenged register: " << TRI->getName(SReg) <<
> "\n");
>      return SReg;
>    }
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation




More information about the llvm-commits mailing list