[llvm] r249087 - AMDGPU: Move SIFixSGPRLiveRanges to be a regalloc pass
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 7 17:39:13 PDT 2015
Matt Arsenault via llvm-commits <llvm-commits at lists.llvm.org> writes:
> Author: arsenm
> Date: Thu Oct 1 17:10:03 2015
> New Revision: 249087
>
> URL: http://llvm.org/viewvc/llvm-project?rev=249087&view=rev
> Log:
> AMDGPU: Move SIFixSGPRLiveRanges to be a regalloc pass
>
> 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.
So SIFixSGPRLiveRanges is causing machine verifier errors after this
commit, but we aren't seeing them because this also accidentally
disabled the running the verifier after the pass, because it turns out
`insertPass` doesn't schedule verification!
I've fixed insertPass in r249643 and explicitly disabled verification
after SIFixSGPRLiveRanges for now - all of the other passes that use
insertPass seem fine. Could you please look into fixing
SIFixSGPRLiveRanges correctly?
> Modified:
> llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
> llvm/trunk/lib/Target/AMDGPU/SIFixSGPRLiveRanges.cpp
>
> Modified: llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp?rev=249087&r1=249086&r2=249087&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp (original)
> +++ llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp Thu Oct 1 17:10:03 2015
> @@ -42,6 +42,9 @@ extern "C" void LLVMInitializeAMDGPUTarg
> // 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) {
> @@ -160,6 +163,8 @@ public:
> : 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;
> @@ -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() {
>
> Modified: llvm/trunk/lib/Target/AMDGPU/SIFixSGPRLiveRanges.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/SIFixSGPRLiveRanges.cpp?rev=249087&r1=249086&r2=249087&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AMDGPU/SIFixSGPRLiveRanges.cpp (original)
> +++ llvm/trunk/lib/Target/AMDGPU/SIFixSGPRLiveRanges.cpp Thu Oct 1 17:10:03 2015
> @@ -80,13 +80,13 @@ public:
> }
>
> 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);
> }
> };
> @@ -95,7 +95,6 @@ public:
>
> 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,
> @@ -117,10 +116,9 @@ bool SIFixSGPRLiveRanges::runOnMachineFu
> 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
> @@ -129,19 +127,22 @@ bool SIFixSGPRLiveRanges::runOnMachineFu
> 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)));
> }
> }
> }
> @@ -169,16 +170,13 @@ bool SIFixSGPRLiveRanges::runOnMachineFu
> *(++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)
> @@ -195,9 +193,9 @@ bool SIFixSGPRLiveRanges::runOnMachineFu
> // 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) &&
> @@ -211,14 +209,7 @@ bool SIFixSGPRLiveRanges::runOnMachineFu
> .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());
> }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list