Skip to content

Commit d05e662

Browse files
authored
[AMDGPU] Handle empty-except-for-DI regions in PreRARematerialize (#516)
[AMDGPU] Handle empty-except-for-DI regions in PreRARematerialize The existing check for this case only comes after a derefence of what can be an iterator sentinel (leading to an assert). This may not be purely NFC in that it also avoids queuing the effectively-empty region for rescheduling, but AFAICT this should be purely an optimization. Testing this seems difficult, as the high-level scheduler avoids scheduling these "empty" regions. This means a reproducer has to depend on behavior of the scheduler passes before PreRARematStage in order to craft a region which triggers the bug. Since this is a release blocker I am posting a PR now, as both Shore Shen and I have manually verified that this resolves the particular crash from SWDEV-564142 but I am still working on making a reasonable test.
2 parents bcb29ee + 004cfea commit d05e662

File tree

1 file changed

+6
-11
lines changed

1 file changed

+6
-11
lines changed

llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2093,7 +2093,9 @@ void PreRARematStage::rematerialize() {
20932093
MF.getInfo<SIMachineFunctionInfo>()->getDynamicVGPRBlockSize();
20942094
AchievedOcc = MFI.getMaxWavesPerEU();
20952095
for (auto &[I, OriginalRP] : ImpactedRegions) {
2096-
bool IsEmptyRegion = DAG.Regions[I].first == DAG.Regions[I].second;
2096+
auto NonDbgMBBI = skipDebugInstructionsForward(DAG.Regions[I].first,
2097+
DAG.Regions[I].second);
2098+
bool IsEmptyRegion = NonDbgMBBI == DAG.Regions[I].second;
20972099
RescheduleRegions[I] = !IsEmptyRegion;
20982100
if (!RecomputeRP.contains(I))
20992101
continue;
@@ -2103,16 +2105,9 @@ void PreRARematStage::rematerialize() {
21032105
RP = getRegPressure(DAG.MRI, DAG.LiveIns[I]);
21042106
} else {
21052107
GCNDownwardRPTracker RPT(*DAG.LIS);
2106-
auto *NonDbgMI = &*skipDebugInstructionsForward(DAG.Regions[I].first,
2107-
DAG.Regions[I].second);
2108-
if (NonDbgMI == DAG.Regions[I].second) {
2109-
// Region is non-empty but contains only debug instructions.
2110-
RP = getRegPressure(DAG.MRI, DAG.LiveIns[I]);
2111-
} else {
2112-
RPT.reset(*NonDbgMI, &DAG.LiveIns[I]);
2113-
RPT.advance(DAG.Regions[I].second);
2114-
RP = RPT.moveMaxPressure();
2115-
}
2108+
RPT.reset(*NonDbgMBBI, &DAG.LiveIns[I]);
2109+
RPT.advance(DAG.Regions[I].second);
2110+
RP = RPT.moveMaxPressure();
21162111
}
21172112
DAG.Pressure[I] = RP;
21182113
AchievedOcc =

0 commit comments

Comments
 (0)