Skip to content

Commit

Permalink
AMDGPU: Move SIFixSGPRLiveRanges to be a regalloc pass
Browse files Browse the repository at this point in the history 
Replace LiveInterval usage with LiveVariables. LiveIntervals
computes far more information than is needed for this pass
which just needs to find if an SGPR is live out of the
defining block.

LiveIntervals are not usually available that early, requiring
computing them twice which is very expensive. The extra run of
LiveIntervals/LiveVariables/SlotIndexes was costing in total
about 5% of compile time.

Continuing to use LiveIntervals is problematic. It seems
there is an option (early-live-intervals) to run the analysis
about where it should go to avoid recomputing LiveVariables,
but it seems to be completely broken with subreg liveness
enabled. There are also problems from trying to recompute
LiveIntervals since this seems to undo LiveVariables
and clearing kill flags, causing TwoAddressInstructions
to make bad decisions.

Insert the pass right after live variables and preserve it.
The tricky case to worry about might be phis since
LiveVariables doesn't count a register as live out if
in the successor block it is only used in a phi,
but I don't think this is a concern right now
because SIFixSGPRCopies replaces SGPR phis.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@249087 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
@arsenm
arsenm committed Oct 1, 2015
1 parent fd81399 commit a7474fd
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 33 deletions.
18 changes: 17 additions & 1 deletion lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ extern "C" void LLVMInitializeAMDGPUTarget() {
// Register the target
RegisterTargetMachine<R600TargetMachine> X(TheAMDGPUTarget);
RegisterTargetMachine<GCNTargetMachine> Y(TheGCNTarget);

PassRegistry *PR = PassRegistry::getPassRegistry();
initializeSIFixSGPRLiveRangesPass(*PR);
}

static std::unique_ptr<TargetLoweringObjectFile> createTLOF(const Triple &TT) {
Expand Down Expand Up @@ -160,6 +163,8 @@ class GCNPassConfig : public AMDGPUPassConfig {
: AMDGPUPassConfig(TM, PM) { }
bool addPreISel() override;
bool addInstSelector() override;
void addFastRegAlloc(FunctionPass *RegAllocPass) override;
void addOptimizedRegAlloc(FunctionPass *RegAllocPass) override;
void addPreRegAlloc() override;
void addPostRegAlloc() override;
void addPreSched2() override;
Expand Down Expand Up @@ -294,7 +299,18 @@ void GCNPassConfig::addPreRegAlloc() {
insertPass(&MachineSchedulerID, &RegisterCoalescerID);
}
addPass(createSIShrinkInstructionsPass(), false);
addPass(createSIFixSGPRLiveRangesPass());
}

void GCNPassConfig::addFastRegAlloc(FunctionPass *RegAllocPass) {
addPass(&SIFixSGPRLiveRangesID);
TargetPassConfig::addFastRegAlloc(RegAllocPass);
}

void GCNPassConfig::addOptimizedRegAlloc(FunctionPass *RegAllocPass) {
// We want to run this after LiveVariables is computed to avoid computing them
// twice.
insertPass(&LiveVariablesID, &SIFixSGPRLiveRangesID);
TargetPassConfig::addOptimizedRegAlloc(RegAllocPass);
}

void GCNPassConfig::addPostRegAlloc() {
Expand Down
55 changes: 23 additions & 32 deletions lib/Target/AMDGPU/SIFixSGPRLiveRanges.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,13 @@ class SIFixSGPRLiveRanges : public MachineFunctionPass {
}

void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.addRequired<LiveIntervals>();
AU.addRequired<LiveVariables>();
AU.addPreserved<LiveVariables>();

AU.addRequired<MachinePostDominatorTree>();
AU.addPreserved<MachinePostDominatorTree>();
AU.setPreservesCFG();

//AU.addPreserved<SlotIndexes>(); // XXX - This might be OK
AU.addPreserved<LiveIntervals>();

MachineFunctionPass::getAnalysisUsage(AU);
}
};
Expand All @@ -95,7 +95,6 @@ class SIFixSGPRLiveRanges : public MachineFunctionPass {

INITIALIZE_PASS_BEGIN(SIFixSGPRLiveRanges, DEBUG_TYPE,
"SI Fix SGPR Live Ranges", false, false)
INITIALIZE_PASS_DEPENDENCY(LiveIntervals)
INITIALIZE_PASS_DEPENDENCY(LiveVariables)
INITIALIZE_PASS_DEPENDENCY(MachinePostDominatorTree)
INITIALIZE_PASS_END(SIFixSGPRLiveRanges, DEBUG_TYPE,
Expand All @@ -117,10 +116,9 @@ bool SIFixSGPRLiveRanges::runOnMachineFunction(MachineFunction &MF) {
bool MadeChange = false;

MachinePostDominatorTree *PDT = &getAnalysis<MachinePostDominatorTree>();
std::vector<std::pair<unsigned, LiveRange *>> SGPRLiveRanges;
SmallVector<unsigned, 16> SGPRLiveRanges;

LiveIntervals *LIS = &getAnalysis<LiveIntervals>();
LiveVariables *LV = getAnalysisIfAvailable<LiveVariables>();
LiveVariables *LV = &getAnalysis<LiveVariables>();
MachineBasicBlock *Entry = MF.begin();

// Use a depth first order so that in SSA, we encounter all defs before
Expand All @@ -129,19 +127,22 @@ bool SIFixSGPRLiveRanges::runOnMachineFunction(MachineFunction &MF) {
for (MachineBasicBlock *MBB : depth_first(Entry)) {
for (const MachineInstr &MI : *MBB) {
for (const MachineOperand &MO : MI.defs()) {
if (MO.isImplicit())
continue;
// We should never see a live out def of a physical register, so we also
// do not need to worry about implicit_defs().
unsigned Def = MO.getReg();
if (TargetRegisterInfo::isVirtualRegister(Def)) {
if (TRI->isSGPRClass(MRI.getRegClass(Def))) {
// Only consider defs that are live outs. We don't care about def /
// use within the same block.
LiveRange &LR = LIS->getInterval(Def);
if (LIS->isLiveOutOfMBB(LR, MBB))
SGPRLiveRanges.push_back(std::make_pair(Def, &LR));

// LiveVariables does not consider registers that are only used in a
// phi in a sucessor block as live out, unlike LiveIntervals.
//
// This is OK because SIFixSGPRCopies replaced any SGPR phis with
// VGPRs.
if (LV->isLiveOut(Def, *MBB))
SGPRLiveRanges.push_back(Def);
}
} else if (TRI->isSGPRClass(TRI->getPhysRegClass(Def))) {
SGPRLiveRanges.push_back(std::make_pair(Def, &LIS->getRegUnit(Def)));
}
}
}
Expand Down Expand Up @@ -169,16 +170,13 @@ bool SIFixSGPRLiveRanges::runOnMachineFunction(MachineFunction &MF) {
*(++NCD->succ_begin()));
}

for (std::pair<unsigned, LiveRange*> RegLR : SGPRLiveRanges) {
unsigned Reg = RegLR.first;
LiveRange *LR = RegLR.second;

for (unsigned Reg : SGPRLiveRanges) {
// FIXME: We could be smarter here. If the register is Live-In to one
// block, but the other doesn't have any SGPR defs, then there won't be a
// conflict. Also, if the branch condition is uniform then there will be
// no conflict.
bool LiveInToA = LIS->isLiveInToMBB(*LR, SuccA);
bool LiveInToB = LIS->isLiveInToMBB(*LR, SuccB);
bool LiveInToA = LV->isLiveIn(Reg, *SuccA);
bool LiveInToB = LV->isLiveIn(Reg, *SuccB);

if (!LiveInToA && !LiveInToB) {
DEBUG(dbgs() << PrintReg(Reg, TRI, 0)
Expand All @@ -195,9 +193,9 @@ bool SIFixSGPRLiveRanges::runOnMachineFunction(MachineFunction &MF) {
// This interval is live in to one successor, but not the other, so
// we need to update its range so it is live in to both.
DEBUG(dbgs() << "Possible SGPR conflict detected for "
<< PrintReg(Reg, TRI, 0) << " in " << *LR
<< " BB#" << SuccA->getNumber() << ", BB#"
<< SuccB->getNumber()
<< PrintReg(Reg, TRI, 0)
<< " BB#" << SuccA->getNumber()
<< ", BB#" << SuccB->getNumber()
<< " with NCD = BB#" << NCD->getNumber() << '\n');

assert(TargetRegisterInfo::isVirtualRegister(Reg) &&
Expand All @@ -211,14 +209,7 @@ bool SIFixSGPRLiveRanges::runOnMachineFunction(MachineFunction &MF) {
.addReg(Reg, RegState::Implicit);

MadeChange = true;

SlotIndex SI = LIS->InsertMachineInstrInMaps(NCDSGPRUse);
LIS->extendToIndices(*LR, SI.getRegSlot());

if (LV) {
// TODO: This won't work post-SSA
LV->HandleVirtRegUse(Reg, NCD, NCDSGPRUse);
}
LV->HandleVirtRegUse(Reg, NCD, NCDSGPRUse);

DEBUG(NCDSGPRUse->dump());
}
Expand Down

0 comments on commit a7474fd

Please sign in to comment.