[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