[llvm] [AMDGPU] Optionally Use GCNRPTrackers during scheduling (PR #93090)
Valery Pykhtin via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 3 03:52:23 PDT 2024
================
@@ -148,17 +154,44 @@ static bool canUsePressureDiffs(const SUnit &SU) {
return true;
}
-static void getRegisterPressures(bool AtTop,
- const RegPressureTracker &RPTracker, SUnit *SU,
- std::vector<unsigned> &Pressure,
- std::vector<unsigned> &MaxPressure) {
+static void getRegisterPressures(
+ bool AtTop, const RegPressureTracker &RPTracker, SUnit *SU,
+ std::vector<unsigned> &Pressure, std::vector<unsigned> &MaxPressure,
+ GCNDownwardRPTracker &DownwardTracker, GCNUpwardRPTracker &UpwardTracker,
+ ScheduleDAGMI *DAG, const SIRegisterInfo *SRI) {
// getDownwardPressure() and getUpwardPressure() make temporary changes to
// the tracker, so we need to pass those function a non-const copy.
RegPressureTracker &TempTracker = const_cast<RegPressureTracker &>(RPTracker);
- if (AtTop)
- TempTracker.getDownwardPressure(SU->getInstr(), Pressure, MaxPressure);
- else
- TempTracker.getUpwardPressure(SU->getInstr(), Pressure, MaxPressure);
+ if (!GCNTrackers) {
+ AtTop
+ ? TempTracker.getDownwardPressure(SU->getInstr(), Pressure, MaxPressure)
+ : TempTracker.getUpwardPressure(SU->getInstr(), Pressure, MaxPressure);
+
+ return;
+ }
+
+ // GCNTrackers
+ Pressure.resize(4, 0);
+ MachineInstr *MI = SU->getInstr();
+ if (AtTop) {
+ GCNDownwardRPTracker TempDownwardTracker(DownwardTracker);
+ TempDownwardTracker.bumpDownwardPressure(MI, SRI);
+ Pressure[AMDGPU::RegisterPressureSets::SReg_32] =
+ TempDownwardTracker.getPressure().getSGPRNum();
+ Pressure[AMDGPU::RegisterPressureSets::VGPR_32] =
+ TempDownwardTracker.getPressure().getArchVGPRNum();
+ Pressure[AMDGPU::RegisterPressureSets::AGPR_32] =
+ TempDownwardTracker.getPressure().getAGPRNum();
+ } else {
+ GCNUpwardRPTracker TempUpwardTracker(UpwardTracker);
----------------
vpykhtin wrote:
So this is the cornerstone of you change. You're keeping two trackers consistent with the order of the scheduling and at every candidate you need to find its impact on the pressure. My concerns are below.
You're making a full copy of tracker object for every candidate at each step. This is not cheap as you're making a deep copy of DenseMap for live register set in every tracker just to throw it away immediately with no use. This would be ok for the start and would be ok if it made the program as simple as:
temp_tracker(consistent_tracker)
temp_tracker.advance/recede
temp_tracker.getPressure
get rid of temp_tracker
But you've created tons of code just to accomplish above. I believe the reason is that LIS isn't consistent with the incoming candidate.
bumpDownwardPressure it's a harder task with non-consistent LIS. Can you do it as a _const_ method of the GCNDownwardTracker? This means that it should not modify the state of the tracker in anyway and preferably with no extra data member to the object itself (except mutables if it's required for some caching). This way it wouldn't require DenseMap copy and make the intention if bumpDownwardPressure clear: It works on a tentative schedule with non-updated LIS.
Upward direction is different. GCNUpwardTracker is created in mind to be used in the situation when LIS isn't updated. The only place where LIS is used by the tracker is the determination of the live mask for register uses. It just reads the mask from the non-updated LIS and the rationale for this is that the defs should dominate uses for any valid schedule and the mask from the non-updated LIS should work. In other words GCNUpwardTracker should be able to be used like the snippet above without additional work.
https://github.com/llvm/llvm-project/pull/93090
More information about the llvm-commits
mailing list