[PATCH] Modify the register scavenger to operate on reg units when checking liveness
Pedro Artigas
partigas at apple.com
Thu Jul 31 16:17:25 PDT 2014
Here is the new version of the patch that addresses the comments by Quentin and Jacob and removes the code for the callee saved bit vector. Compile time measurements indicate no advantage for the changes to tail duplication and the generation of the used bit vector but no regression on average either.
Thanks,
Pedro
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: opensource5.txt
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140731/3eafa73e/attachment.txt>
-------------- next part --------------
Index: include/llvm/CodeGen/MachineRegisterInfo.h
===================================================================
--- include/llvm/CodeGen/MachineRegisterInfo.h (revision 214448)
+++ 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 214448)
+++ 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.
@@ -58,22 +59,20 @@
/// A vector of information on scavenged registers.
SmallVector<ScavengedInfo, 2> Scavenged;
- /// CalleeSavedrRegs - A bitvector of callee saved registers for the target.
- ///
- BitVector CalleeSavedRegs;
+ /// 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;
- /// 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;
-
// 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.
@@ -112,6 +111,9 @@
MachineBasicBlock::iterator getCurrentPosition() const {
return MBBI;
}
+
+ /// isRegUsed - return if a specific register is currently used.
+ bool isRegUsed(int Reg, bool includeReserved);
/// getRegsUsed - return all registers currently in use in used.
void getRegsUsed(BitVector &used, bool includeReserved);
@@ -164,33 +166,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 214448)
+++ 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 214448)
+++ 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 214448)
+++ 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,16 +90,13 @@
// Self-initialize.
if (!MBB) {
NumPhysRegs = TRI->getNumRegs();
- RegsAvailable.resize(NumPhysRegs);
- KillRegs.resize(NumPhysRegs);
- DefRegs.resize(NumPhysRegs);
-
- // Create callee-saved registers bitvector.
- CalleeSavedRegs.resize(NumPhysRegs);
- const MCPhysReg *CSRegs = TRI->getCalleeSavedRegs(&MF);
- if (CSRegs != nullptr)
- for (unsigned i = 0; CSRegs[i]; ++i)
- CalleeSavedRegs.set(CSRegs[i]);
+ TmpRegs.resize(NumPhysRegs);
+
+ NumRegUnits = TRI->getNumRegUnits();
+ RegUnitsAvailable.resize(NumRegUnits);
+ KillRegUnits.resize(NumRegUnits);
+ DefRegUnits.resize(NumRegUnits);
+ TmpRegUnits.resize(NumRegUnits);
}
MBB = mbb;
@@ -104,12 +105,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 +139,42 @@
// 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()) {
+
+ // The code below is needed to obtain a proper set of reg units that are
+ // trully clobbered. As an example consider ARM64 registers X18, X19 and
+ // the "register" W18_W19. The X18 is not preserved while X19 is;
+ // therefore W18_W19 is considered clobbered but it has inside it a reg
+ // unit that is trully preserved (the reg unit for X19). Hence the logic:
+ //
+ // ClobberedRegisterUnits - TrullyPreservedRegisterUnits =
+ // TrullyClobberedRegisterUnits
+ //
+ // Is needed in order to obtain a conservative approximation of the
+ // set that is trully clobbered by the instruction.
+
+ // 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 +186,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 +205,8 @@
determineKillsAndDefs();
// Commit the changes.
- setUsed(KillRegs);
- setUnused(DefRegs);
+ setUsed(KillRegUnits);
+ setUnused(DefRegUnits);
}
if (MBBI == MBB->begin()) {
@@ -223,11 +270,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 +298,35 @@
#endif // NDEBUG
// Commit the changes.
- setUnused(KillRegs);
- setUsed(DefRegs);
+ setUnused(KillRegUnits);
+ setUsed(DefRegUnits);
}
+bool RegScavenger::isRegUsed(int Reg, bool includeReserved) {
+ const BitVector &ReservedRegs = MRI->getReservedRegs();
+ if (includeReserved) {
+ if (ReservedRegs.test(Reg))
+ return true;
+ } else {
+ if (!ReservedRegs.test(Reg))
+ return false;
+ }
+ for (MCRegUnitIterator RUI(Reg, TRI); RUI.isValid(); ++RUI)
+ if (!RegUnitsAvailable.test(*RUI))
+ return true;
+ return false;
+}
+
void RegScavenger::getRegsUsed(BitVector &used, bool includeReserved) {
- used = RegsAvailable;
- used.flip();
+ TmpRegUnits = RegUnitsAvailable;
+ TmpRegUnits.flip();
+ for (int i = TmpRegUnits.find_first();
+ i != -1;
+ i = TmpRegUnits.find_next(i)) {
+ for (MCRegUnitRootIterator RRI(i, TRI); RRI.isValid(); ++RRI)
+ for (MCSuperRegIterator SI(*RRI, TRI, true); SI.isValid(); ++SI)
+ used.set(*SI);
+ }
if (includeReserved)
used |= MRI->getReservedRegs();
else
@@ -259,7 +336,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 +350,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 +452,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 +463,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;
}
Index: lib/CodeGen/TailDuplication.cpp
===================================================================
--- lib/CodeGen/TailDuplication.cpp (revision 214448)
+++ lib/CodeGen/TailDuplication.cpp (working copy)
@@ -798,11 +798,9 @@
RS->enterBasicBlock(PredBB);
if (!PredBB->empty())
RS->forward(std::prev(PredBB->end()));
- BitVector RegsLiveAtExit(TRI->getNumRegs());
- RS->getRegsUsed(RegsLiveAtExit, false);
for (MachineBasicBlock::livein_iterator I = TailBB->livein_begin(),
E = TailBB->livein_end(); I != E; ++I) {
- if (!RegsLiveAtExit[*I])
+ if (!RS->isRegUsed(*I, false))
// If a register is previously livein to the tail but it's not live
// at the end of predecessor BB, then it should be added to its
// livein list.
> On Jul 31, 2014, at 3:21 PM, Pedro Artigas <partigas at apple.com> wrote:
>
> It (the CRS bit vector) is unused, it will be removed once I check in my changes to the scavenger.
>
> Thanks,
>
> Pedro
>
>> On Jul 31, 2014, at 2:35 PM, Jakob Stoklund Olesen <jolesen at apple.com> wrote:
>>
>>
>>> On Jul 31, 2014, at 2:29 PM, Chad Rosier <mcrosier at codeaurora.org> wrote:
>>>
>>>>
>>>> - // 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.
>>
>> Wow, that’s a long time ago, I don’t remember the details.
>>
>> But it should be trivial to determine if the CSR bitvector is used for anything and delete it if it is unused.
>>
>> Thanks,
>> /jakob
>>
>
More information about the llvm-commits
mailing list