[llvm] [AMDGPU] Add and optionally use GCNIterativeRPTrackers (PR #88797)

Jeffrey Byrnes via llvm-commits llvm-commits at lists.llvm.org
Tue May 7 15:02:24 PDT 2024


================
@@ -343,6 +343,48 @@ void GCNRPTracker::reset(const MachineInstr &MI,
   MaxPressure = CurPressure = getRegPressure(*MRI, LiveRegs);
 }
 
+DenseMap<int, GCNRPTracker::LiveRegSet>
----------------
jrbyrnes wrote:

I'm not sure what to do with this comment. 

> I'm not sure what the key is here.

Currently, these maps are keyed on the initial first instructions of the regions. However, if we are going to requery, we need a different key as these first instructions are likely to change. I've used instead RegionIdx -- which tracks the index in the Regions array. This is similarly used to index all these https://github.com/llvm/llvm-project/blob/a5044e6d505deb79f1b00bb39d11096d29b9c910/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp#L623C3-L623C10 I guess we could restructure as vector.

> I'm concerned by building maps for liveness info, that's already encoded in the LiveIntervals. 

While this patch does introduce a LiveOutMap, it doesn't introduce the notion of using a Map to cache live-ins -- this already exists in the code. In this patch, these maps optimize compile time by avoiding recalculation of the live regs. 

> Most of this code is also replicating logic from the generic tracker, and all of this is really hard to get correct.

This is mostly copy-paste from existing code in GCNRegPressure, but, nonetheless, we're running a CQE cycle to root out any correctness / performance issues. Per discussion, I think it's better to slightly duplicate logic for time being, and potentially rework the generic tracker in a long term project.

https://github.com/llvm/llvm-project/pull/88797


More information about the llvm-commits mailing list