-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[AMDGPU] NFC: Add BBLiveOutMap & LiveOut Cache #93089
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
619c2db
9f2f32b
ea70769
a22d7e0
18cf1b9
2fad27e
d4c1c69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -163,13 +163,43 @@ inline raw_ostream &operator<<(raw_ostream &OS, const ScheduleMetrics &Sm) { | |||
return OS; | ||||
} | ||||
|
||||
class GCNScheduleDAGMILive; | ||||
class RegionPressureMap { | ||||
GCNScheduleDAGMILive *DAG; | ||||
// The live in/out pressure as indexed by the first or last MI in the region | ||||
// before scheduling. | ||||
DenseMap<MachineInstr *, GCNRPTracker::LiveRegSet> BBLiveRegMap; | ||||
Comment on lines
+169
to
+171
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm worried this is making the pressure tracking even more complicated and expensive. LiveRegSet already felt wrong, and this adds another level on top of it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is adding an instance of DenseMap<MachineInstr *, GCNRPTracker::LiveRegSet> . However, this PR is not introducing this new level above GCNRPTracker::LiveRegSet, as it already exists in our code
Perhaps more importantly, I don't think there is an alternative to this besides deleting the cache. Implementing a LIS->getLiveRegsAt(SlotIndex) with cached results would probably be counterproductive due to the maintenance required for each call to SlotIndex insertMachineInstrInMaps during scheduling. We could introduce and cache GCNPressureDiffs (instead of the new trackers) which would use our more accurate RP costs to encode the PDiff. But this is ultimately just a different way to build the cache, and I'm not sure this is worth a complete redesign (especially since there may be issues with inconsistent state between tracker and pdiffs). |
||||
// The mapping of RegionIDx to key instruction | ||||
DenseMap<unsigned, MachineInstr *> IdxToInstruction; | ||||
// Whether we are calculating LiveOuts or LiveIns | ||||
bool IsLiveOut; | ||||
// Whether or not the maps have been generated | ||||
bool IsMapGenerated = false; | ||||
|
||||
public: | ||||
RegionPressureMap() {} | ||||
RegionPressureMap(GCNScheduleDAGMILive *GCNDAG, bool LiveOut) | ||||
: DAG(GCNDAG), IsLiveOut(LiveOut) {} | ||||
// Build the Instr->LiveReg and RegionIdx->Instr maps | ||||
void buildLiveRegMap(); | ||||
|
||||
// Retrieve the LiveReg for a given RegionIdx | ||||
GCNRPTracker::LiveRegSet &getLiveRegsForRegionIdx(unsigned RegionIdx) { | ||||
assert(IsMapGenerated); | ||||
assert(IdxToInstruction.find(RegionIdx) != IdxToInstruction.end()); | ||||
MachineInstr *Key = IdxToInstruction[RegionIdx]; | ||||
return BBLiveRegMap[Key]; | ||||
} | ||||
}; | ||||
|
||||
class GCNScheduleDAGMILive final : public ScheduleDAGMILive { | ||||
friend class GCNSchedStage; | ||||
friend class OccInitialScheduleStage; | ||||
friend class UnclusteredHighRPStage; | ||||
friend class ClusteredLowOccStage; | ||||
friend class PreRARematStage; | ||||
friend class ILPInitialScheduleStage; | ||||
friend class RegionPressureMap; | ||||
|
||||
const GCNSubtarget &ST; | ||||
|
||||
|
@@ -215,6 +245,10 @@ class GCNScheduleDAGMILive final : public ScheduleDAGMILive { | |||
|
||||
DenseMap<MachineInstr *, GCNRPTracker::LiveRegSet> getBBLiveInMap() const; | ||||
|
||||
DenseMap<MachineInstr *, GCNRPTracker::LiveRegSet> getBBLiveOutMap() const; | ||||
|
||||
RegionPressureMap RegionLiveOuts; | ||||
jrbyrnes marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
|
||||
// Return current region pressure. | ||||
GCNRegPressure getRealRegPressure(unsigned RegionIdx) const; | ||||
|
||||
|
Uh oh!
There was an error while loading. Please reload this page.