Skip to content

[WIP][CodeGen] Modifying MBB's liveins representation as into regUnits #129847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions llvm/include/llvm/CodeGen/LivePhysRegs.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,12 @@ bool isPhysRegUsedAfter(Register Reg, MachineBasicBlock::iterator MBI);
/// any changes were made.
static inline bool recomputeLiveIns(MachineBasicBlock &MBB) {
LivePhysRegs LPR;
std::vector<MachineBasicBlock::RegisterMaskPair> OldLiveIns;
DenseSet<MCRegister> OldLiveIns;

MBB.clearLiveIns(OldLiveIns);
computeAndAddLiveIns(LPR, MBB);
MBB.sortUniqueLiveIns();

const std::vector<MachineBasicBlock::RegisterMaskPair> &NewLiveIns =
MBB.getLiveIns();
const DenseSet<MCRegister> &NewLiveIns = MBB.getLiveIns();
return OldLiveIns != NewLiveIns;
}

Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/CodeGen/LiveRegUnits.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class LiveRegUnits {

/// Adds register units covered by physical register \p Reg that are
/// part of the lanemask \p Mask.
void addRegMasked(MCRegister Reg, LaneBitmask Mask) {
void addRegMasked(MCRegister Reg, LaneBitmask Mask = LaneBitmask::getAll()) {
for (MCRegUnitMaskIterator Unit(Reg, TRI); Unit.isValid(); ++Unit) {
LaneBitmask UnitMask = (*Unit).second;
if ((UnitMask & Mask).any())
Expand Down
41 changes: 14 additions & 27 deletions llvm/include/llvm/CodeGen/MachineBasicBlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#define LLVM_CODEGEN_MACHINEBASICBLOCK_H

#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/GraphTraits.h"
#include "llvm/ADT/SparseBitVector.h"
#include "llvm/ADT/ilist.h"
Expand Down Expand Up @@ -174,8 +175,7 @@ class MachineBasicBlock
std::optional<uint64_t> IrrLoopHeaderWeight;

/// Keep track of the physical registers that are livein of the basicblock.
using LiveInVector = std::vector<RegisterMaskPair>;
LiveInVector LiveIns;
DenseSet<MCRegister> LiveIns;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's a good idea to change from std::vector to DenseSet. The patch would be less invasive if you just changed it to std:vector<MCRegister>

You should at least benchmark it using http://llvm-compile-time-tracker.com/. You can do this by asking @nikic to add your fork of LLVM, and then any branches you create called "perf/*" will automatically get benchmarked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's a good idea to change from std::vector to DenseSet. The patch would be less invasive if you just changed it to std:vector

It makes sense so aliasing registers (or tuples) will have common regunits (or their roots) to track them uniquely (in set) and check for isLiveIn would be cheaper.

You should at least benchmark it using http://llvm-compile-time-tracker.com/. You can do this by asking @nikic to add your fork of LLVM, and then any branches you create called "perf/*" will automatically get benchmarked.

Sure!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic can you add my forked repo https://github.com/vg0204/llvm-project.git for benchmarking it!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense so aliasing registers (or tuples) will have common regunits (or their roots) to track them uniquely

Right, but the existing code already did that by calling sortUniqueLiveIns after adding all the individual livein registers. Changing to a set might be better, but it should be done as an orthogonal change (either before or after this patch) and it should be benchmarked.


/// Alignment of the basic block. One if the basic block does not need to be
/// aligned.
Expand Down Expand Up @@ -461,28 +461,17 @@ class MachineBasicBlock

// LiveIn management methods.

/// Adds the specified register as a live in. Note that it is an error to add
/// the same register to the same set more than once unless the intention is
/// to call sortUniqueLiveIns after all registers are added.
/// Adds the live regUnits(both of its roots registers) as the live in, based
/// on the LaneMask.
void addLiveIn(MCRegister PhysReg,
LaneBitmask LaneMask = LaneBitmask::getAll()) {
LiveIns.push_back(RegisterMaskPair(PhysReg, LaneMask));
}
void addLiveIn(const RegisterMaskPair &RegMaskPair) {
LiveIns.push_back(RegMaskPair);
}

/// Sorts and uniques the LiveIns vector. It can be significantly faster to do
/// this than repeatedly calling isLiveIn before calling addLiveIn for every
/// LiveIn insertion.
void sortUniqueLiveIns();
LaneBitmask LaneMask = LaneBitmask::getAll());

/// Clear live in list.
void clearLiveIns();

/// Clear the live in list, and return the removed live in's in \p OldLiveIns.
/// Requires that the vector \p OldLiveIns is empty.
void clearLiveIns(std::vector<RegisterMaskPair> &OldLiveIns);
void clearLiveIns(DenseSet<MCRegister> &OldLiveIns);

/// Add PhysReg as live in to this block, and ensure that there is a copy of
/// PhysReg to a virtual register of class RC. Return the virtual register
Expand All @@ -499,7 +488,7 @@ class MachineBasicBlock

// Iteration support for live in sets. These sets are kept in sorted
// order by their register number.
using livein_iterator = LiveInVector::const_iterator;
using livein_iterator = DenseSet<MCRegister>::const_iterator;

/// Unlike livein_begin, this method does not check that the liveness
/// information is accurate. Still for debug purposes it may be useful
Expand All @@ -520,15 +509,15 @@ class MachineBasicBlock
/// Remove entry from the livein set and return iterator to the next.
livein_iterator removeLiveIn(livein_iterator I);

const std::vector<RegisterMaskPair> &getLiveIns() const { return LiveIns; }
const DenseSet<MCRegister> &getLiveIns() const { return LiveIns; }

class liveout_iterator {
public:
using iterator_category = std::input_iterator_tag;
using difference_type = std::ptrdiff_t;
using value_type = RegisterMaskPair;
using pointer = const RegisterMaskPair *;
using reference = const RegisterMaskPair &;
using value_type = MCRegister;
using pointer = const MCRegister *;
using reference = const MCRegister &;

liveout_iterator(const MachineBasicBlock &MBB, MCPhysReg ExceptionPointer,
MCPhysReg ExceptionSelector, bool End)
Expand All @@ -541,8 +530,7 @@ class MachineBasicBlock
LiveRegI = (*BlockI)->livein_begin();
if (!advanceToValidPosition())
return;
if (LiveRegI->PhysReg == ExceptionPointer ||
LiveRegI->PhysReg == ExceptionSelector)
if (*LiveRegI == ExceptionPointer || *LiveRegI == ExceptionSelector)
++(*this);
}
}
Expand All @@ -552,9 +540,8 @@ class MachineBasicBlock
++LiveRegI;
if (!advanceToValidPosition())
return *this;
} while ((*BlockI)->isEHPad() &&
(LiveRegI->PhysReg == ExceptionPointer ||
LiveRegI->PhysReg == ExceptionSelector));
} while ((*BlockI)->isEHPad() && (*LiveRegI == ExceptionPointer ||
*LiveRegI == ExceptionSelector));
return *this;
}

Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ void AggressiveAntiDepBreaker::StartBlock(MachineBasicBlock *BB) {
// Examine the live-in regs of all successors.
for (MachineBasicBlock *Succ : BB->successors())
for (const auto &LI : Succ->liveins()) {
for (MCRegAliasIterator AI(LI.PhysReg, TRI, true); AI.isValid(); ++AI) {
for (MCRegAliasIterator AI(LI, TRI, true); AI.isValid(); ++AI) {
unsigned Reg = *AI;
State->UnionGroups(Reg, 0);
KillIndices[Reg] = BB->size();
Expand Down
7 changes: 1 addition & 6 deletions llvm/lib/CodeGen/BranchFolding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -401,12 +401,7 @@ void BranchFolder::replaceTailWithBranchTo(MachineBasicBlock::iterator OldInst,
// Merging the tails may have switched some undef operand to non-undef ones.
// Add IMPLICIT_DEFS into OldMBB as necessary to have a definition of the
// register.
for (MachineBasicBlock::RegisterMaskPair P : NewDest.liveins()) {
// We computed the liveins with computeLiveIn earlier and should only see
// full registers:
assert(P.LaneMask == LaneBitmask::getAll() &&
"Can only handle full register.");
MCRegister Reg = P.PhysReg;
for (MCRegister Reg : NewDest.liveins()) {
if (!LiveRegs.available(*MRI, Reg))
continue;
DebugLoc DL;
Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/CodeGen/BranchRelaxation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,10 @@ bool BranchRelaxation::fixupUnconditionalBranch(MachineInstr &MI) {

// Add live outs.
for (const MachineBasicBlock *Succ : MBB->successors()) {
for (const MachineBasicBlock::RegisterMaskPair &LiveIn : Succ->liveins())
for (const MCRegister LiveIn : Succ->liveins())
BranchBB->addLiveIn(LiveIn);
}

BranchBB->sortUniqueLiveIns();
BranchBB->addSuccessor(DestBB);
MBB->replaceSuccessor(DestBB, BranchBB);
if (TrampolineInsertionPoint == MBB)
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/CriticalAntiDepBreaker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ void CriticalAntiDepBreaker::StartBlock(MachineBasicBlock *BB) {
// Examine the live-in regs of all successors.
for (const MachineBasicBlock *Succ : BB->successors())
for (const auto &LI : Succ->liveins()) {
for (MCRegAliasIterator AI(LI.PhysReg, TRI, true); AI.isValid(); ++AI) {
for (MCRegAliasIterator AI(LI, TRI, true); AI.isValid(); ++AI) {
MCRegister Reg = *AI;
Classes[Reg.id()] = reinterpret_cast<TargetRegisterClass *>(-1);
KillIndices[Reg.id()] = BBSize;
Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4192,9 +4192,8 @@ bool IRTranslator::runOnMachineFunction(MachineFunction &CurMF) {
EntryBB->end());

// Update the live-in information for the new entry block.
for (const MachineBasicBlock::RegisterMaskPair &LiveIn : EntryBB->liveins())
for (const MCRegister LiveIn : EntryBB->liveins())
NewEntryBB.addLiveIn(LiveIn);
NewEntryBB.sortUniqueLiveIns();

// Get rid of the now empty basic block.
EntryBB->removeSuccessor(&NewEntryBB);
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/LiveIntervals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ void LiveIntervals::computeLiveInRegUnits() {
SlotIndex Begin = Indexes->getMBBStartIdx(&MBB);
LLVM_DEBUG(dbgs() << Begin << "\t" << printMBBReference(MBB));
for (const auto &LI : MBB.liveins()) {
for (MCRegUnit Unit : TRI->regunits(LI.PhysReg)) {
for (MCRegUnit Unit : TRI->regunits(LI)) {
LiveRange *LR = RegUnitRanges[Unit];
if (!LR) {
// Use segment set to speed-up initial computation of the live range.
Expand Down
17 changes: 2 additions & 15 deletions llvm/lib/CodeGen/LivePhysRegs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,21 +153,8 @@ bool LivePhysRegs::available(const MachineRegisterInfo &MRI,

/// Add live-in registers of basic block \p MBB to \p LiveRegs.
void LivePhysRegs::addBlockLiveIns(const MachineBasicBlock &MBB) {
for (const auto &LI : MBB.liveins()) {
MCRegister Reg = LI.PhysReg;
LaneBitmask Mask = LI.LaneMask;
MCSubRegIndexIterator S(Reg, TRI);
assert(Mask.any() && "Invalid livein mask");
if (Mask.all() || !S.isValid()) {
addReg(Reg);
continue;
}
for (; S.isValid(); ++S) {
unsigned SI = S.getSubRegIndex();
if ((Mask & TRI->getSubRegIndexLaneMask(SI)).any())
addReg(S.getSubReg());
}
}
for (const MCRegister Reg : MBB.liveins())
addReg(Reg);
}

/// Adds all callee saved registers to \p LiveRegs.
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/LiveRegUnits.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void LiveRegUnits::accumulate(const MachineInstr &MI) {
static void addBlockLiveIns(LiveRegUnits &LiveUnits,
const MachineBasicBlock &MBB) {
for (const auto &LI : MBB.liveins())
LiveUnits.addRegMasked(LI.PhysReg, LI.LaneMask);
LiveUnits.addRegMasked(LI);
}

/// Adds all callee saved registers to \p LiveUnits.
Expand Down
9 changes: 4 additions & 5 deletions llvm/lib/CodeGen/LiveVariables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -570,9 +570,8 @@ void LiveVariables::runOnBlock(MachineBasicBlock *MBB, unsigned NumRegs) {
// Mark live-in registers as live-in.
SmallVector<Register, 4> Defs;
for (const auto &LI : MBB->liveins()) {
assert(LI.PhysReg.isPhysical() &&
"Cannot have a live-in virtual register!");
HandlePhysRegDef(LI.PhysReg, nullptr, Defs);
assert(LI.isPhysical() && "Cannot have a live-in virtual register!");
HandlePhysRegDef(LI, nullptr, Defs);
}

// Loop over all of the instructions, processing them.
Expand Down Expand Up @@ -606,9 +605,9 @@ void LiveVariables::runOnBlock(MachineBasicBlock *MBB, unsigned NumRegs) {
if (SuccMBB->isEHPad())
continue;
for (const auto &LI : SuccMBB->liveins()) {
if (!TRI->isInAllocatableClass(LI.PhysReg))
if (!TRI->isInAllocatableClass(LI))
// Ignore other live-ins, e.g. those that are live into landing pads.
LiveOuts.insert(LI.PhysReg);
LiveOuts.insert(LI);
}
}

Expand Down
4 changes: 1 addition & 3 deletions llvm/lib/CodeGen/MIRPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -730,9 +730,7 @@ void printMBB(raw_ostream &OS, MFPrintState &State,
OS.indent(2) << "liveins: ";
ListSeparator LS;
for (const auto &LI : MBB.liveins_dbg()) {
OS << LS << printReg(LI.PhysReg, &TRI);
if (!LI.LaneMask.all())
OS << ":0x" << PrintLaneMask(LI.LaneMask);
OS << LS << printReg(LI, &TRI);
}
OS << "\n";
HasLineAttributes = true;
Expand Down
Loading
Loading