[AMDGPU] Add SSA-form memory clause pass (AMDGPUFormSSAMemoryClauses) - #209656
[AMDGPU] Add SSA-form memory clause pass (AMDGPUFormSSAMemoryClauses)#209656jwanggit86 wants to merge 8 commits into
Conversation
|
@llvm/pr-subscribers-backend-amdgpu Author: Jun Wang (jwanggit86) ChangesCreate a new backend pass for AMDGPU named SSASIFormMemoryClauses. This pass does the same work as Patch is 34.23 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/209656.diff 6 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPU.h b/llvm/lib/Target/AMDGPU/AMDGPU.h
index c6dd1dbb62449..bf52cd32b1a34 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPU.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPU.h
@@ -55,6 +55,7 @@ FunctionPass *createSIMemoryLegalizerPass();
FunctionPass *createSIInsertWaitcntsPass();
FunctionPass *createSIPreAllocateWWMRegsLegacyPass();
FunctionPass *createSIFormMemoryClausesLegacyPass();
+FunctionPass *createSSASIFormMemoryClausesLegacyPass();
FunctionPass *createSIPostRABundlerPass();
FunctionPass *createAMDGPUImageIntrinsicOptimizerPass(const TargetMachine *);
@@ -551,6 +552,9 @@ extern char &SIInsertWaitcntsID;
void initializeSIFormMemoryClausesLegacyPass(PassRegistry &);
extern char &SIFormMemoryClausesID;
+void initializeSSASIFormMemoryClausesLegacyPass(PassRegistry &);
+extern char &SSASIFormMemoryClausesID;
+
void initializeSIPostRABundlerLegacyPass(PassRegistry &);
extern char &SIPostRABundlerLegacyID;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index ae6e6d0bdcd1e..a34a69898ad31 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -53,6 +53,7 @@
#include "SIFixVGPRCopies.h"
#include "SIFoldOperands.h"
#include "SIFormMemoryClauses.h"
+#include "SSASIFormMemoryClauses.h"
#include "SILoadStoreOptimizer.h"
#include "SILowerControlFlow.h"
#include "SILowerSGPRSpills.h"
@@ -564,6 +565,11 @@ static cl::opt<bool> EnablePreRAOptimizations(
cl::desc("Enable Pre-RA optimizations pass"), cl::init(true),
cl::Hidden);
+static cl::opt<bool> EnableSSASIFormMemoryClauses(
+ "amdgpu-enable-ssa-form-memory-clauses",
+ cl::desc("Enable SSA form memory clause pass (before PHI elimination)"),
+ cl::init(false), cl::Hidden);
+
static cl::opt<bool> EnablePromoteKernelArguments(
"amdgpu-enable-promote-kernel-arguments",
cl::desc("Enable promotion of flat kernel pointer arguments to global"),
@@ -714,6 +720,7 @@ extern "C" LLVM_ABI LLVM_EXTERNAL_VISIBILITY void LLVMInitializeAMDGPUTarget() {
initializeSIOptimizeExecMaskingLegacyPass(*PR);
initializeSIPreAllocateWWMRegsLegacyPass(*PR);
initializeSIFormMemoryClausesLegacyPass(*PR);
+ initializeSSASIFormMemoryClausesLegacyPass(*PR);
initializeSIPostRABundlerLegacyPass(*PR);
initializeGCNCreateVOPDLegacyPass(*PR);
initializeAMDGPUUnifyDivergentExitNodesLegacyPass(*PR);
@@ -1413,6 +1420,7 @@ AMDGPUPassConfig::AMDGPUPassConfig(TargetMachine &TM, PassManagerBase &PM)
// Garbage collection is not supported.
disablePass(&GCLoweringID);
disablePass(&ShadowStackGCLoweringID);
+
}
void AMDGPUPassConfig::addEarlyCSEOrGVNPass() {
@@ -1758,9 +1766,17 @@ void GCNPassConfig::addOptimizedRegAlloc() {
// This is not an essential optimization and it has a noticeable impact on
// compilation time, so we only enable it from O2.
- if (TM->getOptLevel() > CodeGenOptLevel::Less)
+ if (TM->getOptLevel() > CodeGenOptLevel::Less && !EnableSSASIFormMemoryClauses)
insertPass(&MachineSchedulerID, &SIFormMemoryClausesID);
+ // Run the SSA form of the memory clause pass before PHI elimination.
+ // LiveVariables is the anchor: it runs in SSA form and sets kill flags that
+ // our pass relies on for intra-block liveness tracking.
+ // TODO: Once PR #161054 (SSAMachineScheduler) is merged, anchor this pass
+ // after SSAMachineSchedulerID instead of LiveVariablesID.
+ if (EnableSSASIFormMemoryClauses)
+ insertPass(&LiveVariablesID, &SSASIFormMemoryClausesID);
+
TargetPassConfig::addOptimizedRegAlloc();
}
diff --git a/llvm/lib/Target/AMDGPU/CMakeLists.txt b/llvm/lib/Target/AMDGPU/CMakeLists.txt
index 46edc44e2cc05..8273851731b17 100644
--- a/llvm/lib/Target/AMDGPU/CMakeLists.txt
+++ b/llvm/lib/Target/AMDGPU/CMakeLists.txt
@@ -163,6 +163,7 @@ add_llvm_target(AMDGPUCodeGen
SIFixVGPRCopies.cpp
SIFoldOperands.cpp
SIFormMemoryClauses.cpp
+ SSASIFormMemoryClauses.cpp
SIFrameLowering.cpp
SIInsertHardClauses.cpp
SIInsertWaitcnts.cpp
diff --git a/llvm/lib/Target/AMDGPU/SSASIFormMemoryClauses.cpp b/llvm/lib/Target/AMDGPU/SSASIFormMemoryClauses.cpp
new file mode 100644
index 0000000000000..944dcc7a2215e
--- /dev/null
+++ b/llvm/lib/Target/AMDGPU/SSASIFormMemoryClauses.cpp
@@ -0,0 +1,468 @@
+//===-- SSASIFormMemoryClauses.cpp ----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+/// \file This pass is a clone of SIFormMemoryClauses intended to run in SSA
+/// form, before PHI elimination. It extends the live ranges of registers used
+/// as pointers in sequences of adjacent SMEM and VMEM instructions when XNACK
+/// is enabled, preventing a load from overwriting a pointer and requiring a
+/// soft clause break.
+///
+/// TODO: Once PR #161054 (SSAMachineScheduler) is merged this pass should be
+/// placed immediately after SSAMachineScheduler in the pipeline.
+///
+//===----------------------------------------------------------------------===//
+
+#include "SSASIFormMemoryClauses.h"
+#include "AMDGPU.h"
+#include "GCNRegPressure.h"
+#include "SIMachineFunctionInfo.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/CodeGen/LiveVariables.h"
+#include "llvm/InitializePasses.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "ssa-si-form-memory-clauses"
+
+// Clauses longer then 15 instructions would overflow one of the counters
+// and stall. They can stall even earlier if there are outstanding counters.
+static cl::opt<unsigned>
+SSAMaxClause("amdgpu-ssa-max-memory-clause", cl::Hidden, cl::init(15),
+ cl::desc("Maximum length of a memory clause for SSA form pass, "
+ "instructions"));
+
+namespace {
+
+class SSASIFormMemoryClausesImpl {
+ using RegUse = DenseMap<unsigned, std::pair<RegState, LaneBitmask>>;
+
+ bool canBundle(const MachineInstr &MI, const RegUse &Defs,
+ const RegUse &Uses) const;
+ bool checkPressure(const MachineInstr &MI, GCNRegPressure &CurPressure);
+ void collectRegUses(const MachineInstr &MI, RegUse &Defs,
+ RegUse &Uses) const;
+ bool processRegUses(const MachineInstr &MI, RegUse &Defs, RegUse &Uses,
+ GCNRegPressure &CurPressure);
+
+ const GCNSubtarget *ST;
+ const SIRegisterInfo *TRI;
+ const MachineRegisterInfo *MRI;
+ SIMachineFunctionInfo *MFI;
+ LiveVariables *LV;
+
+ unsigned LastRecordedOccupancy;
+ unsigned MaxVGPRs;
+ unsigned MaxSGPRs;
+
+public:
+ bool run(MachineFunction &MF, LiveVariables &LV);
+};
+
+class SSASIFormMemoryClausesLegacy : public MachineFunctionPass {
+public:
+ static char ID;
+
+ SSASIFormMemoryClausesLegacy() : MachineFunctionPass(ID) {}
+
+ bool runOnMachineFunction(MachineFunction &MF) override;
+
+ StringRef getPassName() const override {
+ return "SSA SI Form memory clauses";
+ }
+
+ void getAnalysisUsage(AnalysisUsage &AU) const override {
+ AU.addRequired<LiveVariablesWrapperPass>();
+ AU.setPreservesAll();
+ MachineFunctionPass::getAnalysisUsage(AU);
+ }
+
+ // Unlike SIFormMemoryClauses, we do NOT clear the IsSSA property because
+ // this pass is designed to run while the function is still in SSA form.
+};
+
+} // End anonymous namespace.
+
+INITIALIZE_PASS_BEGIN(SSASIFormMemoryClausesLegacy, DEBUG_TYPE,
+ "SSA SI Form memory clauses", false, false)
+INITIALIZE_PASS_DEPENDENCY(LiveVariablesWrapperPass)
+INITIALIZE_PASS_END(SSASIFormMemoryClausesLegacy, DEBUG_TYPE,
+ "SSA SI Form memory clauses", false, false)
+
+char SSASIFormMemoryClausesLegacy::ID = 0;
+
+char &llvm::SSASIFormMemoryClausesID = SSASIFormMemoryClausesLegacy::ID;
+
+FunctionPass *llvm::createSSASIFormMemoryClausesLegacyPass() {
+ return new SSASIFormMemoryClausesLegacy();
+}
+
+static bool isVMEMClauseInst(const MachineInstr &MI) {
+ return SIInstrInfo::isVMEM(MI);
+}
+
+static bool isSMEMClauseInst(const MachineInstr &MI) {
+ return SIInstrInfo::isSMRD(MI);
+}
+
+// There no sense to create store clauses, they do not define anything,
+// thus there is nothing to set early-clobber.
+static bool isValidClauseInst(const MachineInstr &MI, bool IsVMEMClause) {
+ assert(!MI.isDebugInstr() && "debug instructions should not reach here");
+ if (MI.isBundled())
+ return false;
+ if (!MI.mayLoad() || MI.mayStore())
+ return false;
+ if (SIInstrInfo::isAtomic(MI))
+ return false;
+ if (IsVMEMClause && !isVMEMClauseInst(MI))
+ return false;
+ if (!IsVMEMClause && !isSMEMClauseInst(MI))
+ return false;
+ // If this is a load instruction where the result has been coalesced with an
+ // operand, then we cannot clause it.
+ for (const MachineOperand &ResMO : MI.defs()) {
+ Register ResReg = ResMO.getReg();
+ for (const MachineOperand &MO : MI.all_uses()) {
+ if (MO.getReg() == ResReg)
+ return false;
+ }
+ break; // Only check the first def.
+ }
+ return true;
+}
+
+static RegState getMopState(const MachineOperand &MO) {
+ RegState S = {};
+ if (MO.isImplicit())
+ S |= RegState::Implicit;
+ if (MO.isDead())
+ S |= RegState::Dead;
+ if (MO.isUndef())
+ S |= RegState::Undef;
+ if (MO.isKill())
+ S |= RegState::Kill;
+ if (MO.isEarlyClobber())
+ S |= RegState::EarlyClobber;
+ if (MO.getReg().isPhysical() && MO.isRenamable())
+ S |= RegState::Renamable;
+ return S;
+}
+
+// Returns false if there is a use of a def already in the map.
+// In this case we must break the clause.
+bool SSASIFormMemoryClausesImpl::canBundle(const MachineInstr &MI,
+ const RegUse &Defs,
+ const RegUse &Uses) const {
+ // Check interference with defs.
+ for (const MachineOperand &MO : MI.operands()) {
+ // TODO: Prologue/Epilogue Insertion pass does not process bundled
+ // instructions.
+ if (MO.isFI())
+ return false;
+
+ if (!MO.isReg())
+ continue;
+
+ Register Reg = MO.getReg();
+
+ // If it is tied we will need to write same register as we read.
+ if (MO.isTied())
+ return false;
+
+ const RegUse &Map = MO.isDef() ? Uses : Defs;
+ auto Conflict = Map.find(Reg);
+ if (Conflict == Map.end())
+ continue;
+
+ if (Reg.isPhysical())
+ return false;
+
+ LaneBitmask Mask = TRI->getSubRegIndexLaneMask(MO.getSubReg());
+ if ((Conflict->second.second & Mask).any())
+ return false;
+ }
+
+ return true;
+}
+
+// Since all defs in the clause are early clobber we can run out of registers.
+// Function returns false if pressure would hit the limit if instruction is
+// bundled into a memory clause.
+//
+// We accumulate pressure monotonically across the clause: because all defs are
+// marked early-clobber they remain live until the clause end, so we never
+// subtract pressure for uses that die mid-clause. This is conservative and
+// avoids the need for LiveIntervals.
+bool SSASIFormMemoryClausesImpl::checkPressure(const MachineInstr &MI,
+ GCNRegPressure &CurPressure) {
+ // Speculatively add this instruction's virtual defs to the running pressure.
+ // Physical register defs are skipped: they are not allocatable slots and
+ // GCNRegPressure::inc() requires a virtual register.
+ GCNRegPressure NewPressure = CurPressure;
+ for (const MachineOperand &MO : MI.defs()) {
+ if (!MO.isReg() || !MO.getReg().isVirtual())
+ continue;
+ Register Reg = MO.getReg();
+ LaneBitmask Mask = MO.getSubReg()
+ ? TRI->getSubRegIndexLaneMask(MO.getSubReg())
+ : MRI->getMaxLaneMaskForVReg(Reg);
+ NewPressure.inc(Reg, LaneBitmask::getNone(), Mask, *MRI);
+ }
+
+ unsigned Occupancy = NewPressure.getOccupancy(
+ *ST,
+ MI.getMF()->getInfo<SIMachineFunctionInfo>()->getDynamicVGPRBlockSize());
+
+ // Don't push over half the register budget. We don't want to introduce
+ // spilling just to form a soft clause.
+ //
+ // FIXME: This pressure check is fundamentally broken. First, this is checking
+ // the global pressure, not the pressure at this specific point in the
+ // program. Second, it's not accounting for the increased liveness of the use
+ // operands due to the early clobber we will introduce. Third, the pressure
+ // tracking does not account for the alignment requirements for SGPRs, or the
+ // fragmentation of registers the allocator will need to satisfy.
+ if (Occupancy >= MFI->getMinAllowedOccupancy() &&
+ NewPressure.getVGPRNum(ST->hasGFX90AInsts()) <= MaxVGPRs / 2 &&
+ NewPressure.getSGPRNum() <= MaxSGPRs / 2) {
+ LastRecordedOccupancy = Occupancy;
+ CurPressure = NewPressure;
+ return true;
+ }
+ return false;
+}
+
+// Collect register defs and uses along with their lane masks and states.
+void SSASIFormMemoryClausesImpl::collectRegUses(const MachineInstr &MI,
+ RegUse &Defs,
+ RegUse &Uses) const {
+ for (const MachineOperand &MO : MI.operands()) {
+ if (!MO.isReg())
+ continue;
+ Register Reg = MO.getReg();
+ if (!Reg)
+ continue;
+
+ LaneBitmask Mask = Reg.isVirtual()
+ ? TRI->getSubRegIndexLaneMask(MO.getSubReg())
+ : LaneBitmask::getAll();
+ RegUse &Map = MO.isDef() ? Defs : Uses;
+
+ RegState State = getMopState(MO);
+ auto [Loc, Inserted] = Map.try_emplace(Reg, State, Mask);
+ if (!Inserted) {
+ Loc->second.first |= State;
+ Loc->second.second |= Mask;
+ }
+ }
+}
+
+// Check register def/use conflicts, occupancy limits and collect def/use maps.
+// Return true if instruction can be bundled with previous. If it cannot
+// def/use maps are not updated.
+bool SSASIFormMemoryClausesImpl::processRegUses(const MachineInstr &MI,
+ RegUse &Defs, RegUse &Uses,
+ GCNRegPressure &CurPressure) {
+ if (!canBundle(MI, Defs, Uses))
+ return false;
+
+ if (!checkPressure(MI, CurPressure))
+ return false;
+
+ collectRegUses(MI, Defs, Uses);
+ return true;
+}
+
+bool SSASIFormMemoryClausesImpl::run(MachineFunction &MF, LiveVariables &LVIn) {
+ ST = &MF.getSubtarget<GCNSubtarget>();
+ if (!ST->isXNACKEnabled())
+ return false;
+
+ const SIInstrInfo *TII = ST->getInstrInfo();
+ TRI = ST->getRegisterInfo();
+ MRI = &MF.getRegInfo();
+ MFI = MF.getInfo<SIMachineFunctionInfo>();
+ LV = &LVIn;
+ bool Changed = false;
+
+ MaxVGPRs = TRI->getAllocatableSet(MF, &AMDGPU::VGPR_32RegClass).count();
+ MaxSGPRs = TRI->getAllocatableSet(MF, &AMDGPU::SGPR_32RegClass).count();
+ unsigned FuncMaxClause = MF.getFunction().getFnAttributeAsParsedInteger(
+ "amdgpu-max-memory-clause", SSAMaxClause);
+
+ for (MachineBasicBlock &MBB : MF) {
+ // BlockPressure tracks the register pressure at the current scan position
+ // within MBB. It is seeded with virtual registers live-in to this block
+ // (as computed by LiveVariables), then updated instruction-by-instruction:
+ // virtual register defs increase pressure; uses with kill flags decrease
+ // it. In SSA form, kill flags are reliable (each vreg has exactly one
+ // def), so this gives accurate intra-block liveness.
+ GCNRegPressure BlockPressure;
+ for (unsigned I = 0, E = MRI->getNumVirtRegs(); I != E; ++I) {
+ Register Reg = Register::index2VirtReg(I);
+ if (LV->isLiveIn(Reg, MBB)) {
+ LaneBitmask Mask = MRI->getMaxLaneMaskForVReg(Reg);
+ BlockPressure.inc(Reg, LaneBitmask::getNone(), Mask, *MRI);
+ }
+ }
+
+ // PressurePos is the next instruction to be consumed into BlockPressure.
+ // It may lag behind the outer loop iterator when the inner clause-extension
+ // loop advances Next past instructions not admitted to a clause.
+ // advanceBlockPressure() catches it up before each clause attempt.
+ auto PressurePos = MBB.instr_begin();
+
+ auto advanceBlockPressure = [&](MachineBasicBlock::instr_iterator Target) {
+ while (PressurePos != Target) {
+ const MachineInstr &CurMI = *PressurePos++;
+ if (CurMI.isMetaInstruction())
+ continue;
+ for (const MachineOperand &MO : CurMI.operands()) {
+ if (!MO.isReg() || !MO.getReg().isVirtual())
+ continue;
+ Register Reg = MO.getReg();
+ LaneBitmask Mask = TRI->getSubRegIndexLaneMask(MO.getSubReg());
+ if (MO.isDef())
+ BlockPressure.inc(Reg, LaneBitmask::getNone(), Mask, *MRI);
+ else if (MO.isKill())
+ BlockPressure.inc(Reg, Mask, LaneBitmask::getNone(), *MRI);
+ }
+ }
+ };
+
+ MachineBasicBlock::instr_iterator Next;
+ for (auto I = MBB.instr_begin(), E = MBB.instr_end(); I != E; I = Next) {
+ MachineInstr &MI = *I;
+ Next = std::next(I);
+
+ if (MI.isMetaInstruction())
+ continue;
+
+ bool IsVMEM = isVMEMClauseInst(MI);
+
+ if (!isValidClauseInst(MI, IsVMEM)) {
+ advanceBlockPressure(Next);
+ continue;
+ }
+
+ // Bring BlockPressure up to (but not including) MI, then snapshot it as
+ // the baseline pressure entering this potential clause.
+ advanceBlockPressure(I);
+ GCNRegPressure CurPressure = BlockPressure;
+
+ RegUse Defs, Uses;
+ // Kills: virtual registers with isKill() on any use inside the clause.
+ // These registers die within the clause and need a whole-register KILL
+ // pseudo after the last load to extend their live range past the
+ // early-clobber defs. The specific subreg that LV flagged does not
+ // matter; we always emit a whole-register KILL.
+ DenseSet<Register> Kills;
+
+ auto collectKills = [&](const MachineInstr &Instr) {
+ for (const MachineOperand &MO : Instr.operands()) {
+ if (!MO.isReg() || MO.isDef() || !MO.isKill() ||
+ !MO.getReg().isVirtual())
+ continue;
+ Kills.insert(MO.getReg());
+ }
+ };
+
+ if (!processRegUses(MI, Defs, Uses, CurPressure)) {
+ advanceBlockPressure(Next);
+ continue;
+ }
+ collectKills(MI);
+
+ MachineBasicBlock::instr_iterator LastClauseInst = Next;
+ unsigned Length = 1;
+ for (; Next != E && Length < FuncMaxClause; ++Next) {
+ // Debug instructions should not change the kill insertion.
+ if (Next->isMetaInstruction())
+ continue;
+
+ if (!isValidClauseInst(*Next, IsVMEM))
+ break;
+
+ // A load from pointer which was loaded inside the same bundle is an
+ // impossible clause because we will need to write and read the same
+ // register inside. In this case processRegUses will return false.
+ if (!processRegUses(*Next, Defs, Uses, CurPressure))
+ break;
+
+ collectKills(*Next);
+ LastClauseInst = Next;
+ ++Length;
+ }
+ if (Length < 2) {
+ // Clause did not form; process MI normally. Instructions examined by
+ // the inner loop but not admitted will be caught up by
+ // advanceBlockPressure() at the start of the next outer iteration.
+ advanceBlockPressure(std::next(I));
+ continue;
+ }
+
+ Changed = true;
+ MFI->limitOccupancy(LastRecordedOccupancy);
+
+ assert(!LastClauseInst->isMetaInstruction());
+
+ // For each register killed within the clause, insert a whole-register
+ // KILL pseudo after the clause to extend its liveness through the
+ // early-clobber defs. Registers not in Kills are live past the clause
+ // and need nothing.
+ for (Register Reg : Kills) {
+ auto UseIt = Uses.find(Reg);
+ assert(UseIt != Uses.end());
+ RegState UseState = UseIt->second.first & ~RegState::Kill;
+
+ MachineInstrBuilder Kill =
+ BuildMI(*MI.getParent(), std::next(LastClauseInst), DebugLoc(),
+ TII->get(AMDGPU::KILL));
+ Kill.addUse(Reg, UseState | RegState::Kill, AMDGPU::NoSubRegister);
+
+ // Move the kill record ...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
5a33a2e to
668aa8d
Compare
|
Can you explain the motivation for this a bit more? I was under the impression that Also I would expect this to be called something like |
There was a problem hiding this comment.
New LiveVariables uses should not be introduced
There was a problem hiding this comment.
So if a pass needs LiveVars, what's the alternative?
There was a problem hiding this comment.
LiveIntervals is after PHI elimination. This pass needs SSA.
There was a problem hiding this comment.
LiveIntervals can exist prior to PHI Elimination.
I put in a few change in the past to ensure they were preserved when generated earlier, primarily to facilitate #161054.
There was a problem hiding this comment.
LiveIntervals can exist prior to PHI Elimination. I put in a few change in the past to ensure they were preserved when generated earlier, primarily to facilitate #161054.
@perlfu You mean running LiveIntervals prior to PHI Elimination but preserving the results so it is not necessary to run it again at its original post-PHI-Elimination point? Is there a PR for this?
There was a problem hiding this comment.
The PR I mentioned contains code to add an early invocation of PHI Elimination.
If you check PHI Elimination it already supports LiveInterval preservation, previously I did work on this related to AMDGPU in #69429.
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
31b9723 to
c1ea1be
Compare
live interval analysis which is a post-SSA pass.
…l, which may not be correct
Previously block pressures are initialized to zero. This is not ideal. Making use of the analysis done by Live Vars (an SSA pass) we can make the initial block pressure more accurate.
c1ea1be to
1f6ae20
Compare
jwanggit86
changed the title
Done. |
|
|
||
| #define DEBUG_TYPE "amdgpu-form-ssa-memory-clauses" | ||
|
|
||
| // Clauses longer then 15 instructions would overflow one of the counters |
There was a problem hiding this comment.
... hold on, is this actually true? On which architectures? IIRC if there's hard clausing you can go to 64 instructions / s_clause 63 except that there's the bug on some hardware limiting you to 32
There was a problem hiding this comment.
This is copied from SIFormMemoryClauses. The original comment says some counter would overflow if the length is over 15, but it does not say which counter. Could it be the vmcnt of waitcnt? That counter was 4-bit, but in later generations enlarged to 6.
The s_clause instruction you mentioned is so-called hard clause. This pass deals with creating soft clause for XANCK enabled targets.
There was a problem hiding this comment.
Just double-checking here that the feature matrix here is xnack [xor] hard clauses? Because that doesn't feel right.
I'd also consider whether hard clauses should be done by the same mechanism?
|
|
||
| bool AMDGPUFormSSAMemoryClausesImpl::run(MachineFunction &MF) { | ||
| ST = &MF.getSubtarget<GCNSubtarget>(); | ||
| if (!ST->isXNACKEnabled()) |
There was a problem hiding this comment.
Why is this keyed off xnack?
There was a problem hiding this comment.
Because the pass (same as SIFormMemoryClauses) deals with creating soft clauses for XNACK enabled targets.
| return new AMDGPUFormSSAMemoryClausesLegacy(); | ||
| } | ||
|
|
||
| static bool isVMEMClauseInst(const MachineInstr &MI) { |
There was a problem hiding this comment.
Most of the static functions are the same as their counterparts in SIFormMemoryClauses.cpp. Factor them out into a common design for better code maintenance. Check if they qualify for SIInstrInfo. Otherwise, make a MemClauseUtils file similar to MemoryUtils or WaitcntUtils.
There was a problem hiding this comment.
Or just define both in the same file
There was a problem hiding this comment.
The common code is now extracted and put in a separate file: AMDGPUFormSSAMemoryClausesImpl.cpp.
|
This PR duplicates roughly 430 lines from SIFormMemoryClauses.cpp verbatim. Since the old and new passes are expected to coexist for quite some time, have you considered sharing the core implementation and making each pass just a thin wrapper around the SSA-specific differences, such as clearing isSSA and processing PHIs? That would save us from having to keep two copies in sync. |
| #include "llvm/CodeGen/MachinePassManager.h" | ||
|
|
||
| namespace llvm { | ||
| class AMDGPUFormSSAMemoryClausesPass |
There was a problem hiding this comment.
If you added the new PM support - make it working by adding to AMDGPUPassRegistry.def and the -passes=<your_pass_name> to ensure it works.
There was a problem hiding this comment.
Added to AMDGPUPassRegistry.def.
|
|
||
| // Clauses longer then 15 instructions would overflow one of the counters | ||
| // and stall. They can stall even earlier if there are outstanding counters. | ||
| static cl::opt<unsigned> SSAMaxClause( |
There was a problem hiding this comment.
Please don't split the options. Use the existing "amdgpu-max-memory-clause" option instead. It does not depend on SSA.
Add pass to AMDGPUPassRegistry.def. Also change anchoring point to UnreachableMachineBlockElimPass.
The common implementation is now in a separate file. |
6 participants
Create a new backend pass for AMDGPU named AMDGPUFormSSAMemoryClauses. This pass does the same work as
the SIFormMemoryClause pass, i.e., for targets with XNACK enabled, it creates memory clauses out of consecutive
SMEM/VMEM load instructions and extends live ranges of registers killed within the clause to the end of the clause.
The key difference from SIFormMemoryClause is that the new pass runs before PHI elimination.