[AMDGPU] Fixed crash in getLastMIForRegion when the region is empty. (#168653)
PreRARematStage builds region live-outs if GCN trackers are enabled. If rematerialization leads to empty regions, this can cause a crash because of dereference of an invalid iterator in getLastMIForRegion. The fix is to skip calling getLastMIForRegion for empty regions. This patch fixes another bug in the same code region. getLastMIForRegion calls skipDebugInstructionsBackward which may immediately return the RegionEnd if it is not the begin instruction and it is a non-debug instruction. That would imply considering an instruction that is outside the relevant region. The fix is to always pass the previous of RegionEnd to skipDebugInstructionsBackward. This bug was found while using GCN trackers on the existing LIT test machine-scheduler-sink-trivial-remats.mir. Here's the assertion failure. llvm-project/llvm/include/llvm/ADT/ilist_iterator.h:168: llvm::ilist_iterator<OptionsT, IsReverse, IsConst>::reference llvm::ilist_iterator<OptionsT, IsReverse, IsConst>::operator*() const [with OptionsT = llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void, false, void>; bool IsReverse = false; bool IsConst = false; llvm::ilist_iterator<OptionsT, IsReverse, IsConst>::reference = llvm::MachineInstr&]: Assertion `!NodePtr->isKnownSentinel()' failed.
This commit is contained in:
parent
af73aeaa19
commit
94e4ee38aa
@ -978,10 +978,8 @@ GCNScheduleDAGMILive::getRealRegPressure(unsigned RegionIdx) const {
|
||||
|
||||
static MachineInstr *getLastMIForRegion(MachineBasicBlock::iterator RegionBegin,
|
||||
MachineBasicBlock::iterator RegionEnd) {
|
||||
auto REnd = RegionEnd == RegionBegin->getParent()->end()
|
||||
? std::prev(RegionEnd)
|
||||
: RegionEnd;
|
||||
return &*skipDebugInstructionsBackward(REnd, RegionBegin);
|
||||
assert(RegionBegin != RegionEnd && "Region must not be empty");
|
||||
return &*skipDebugInstructionsBackward(std::prev(RegionEnd), RegionBegin);
|
||||
}
|
||||
|
||||
void GCNScheduleDAGMILive::computeBlockPressure(unsigned RegionIdx,
|
||||
@ -1076,9 +1074,12 @@ GCNScheduleDAGMILive::getRegionLiveOutMap() const {
|
||||
assert(!Regions.empty());
|
||||
std::vector<MachineInstr *> RegionLastMIs;
|
||||
RegionLastMIs.reserve(Regions.size());
|
||||
for (auto &[RegionBegin, RegionEnd] : reverse(Regions))
|
||||
for (auto &[RegionBegin, RegionEnd] : reverse(Regions)) {
|
||||
// Skip empty regions.
|
||||
if (RegionBegin == RegionEnd)
|
||||
continue;
|
||||
RegionLastMIs.push_back(getLastMIForRegion(RegionBegin, RegionEnd));
|
||||
|
||||
}
|
||||
return getLiveRegMap(RegionLastMIs, /*After=*/true, *LIS);
|
||||
}
|
||||
|
||||
@ -1088,10 +1089,12 @@ void RegionPressureMap::buildLiveRegMap() {
|
||||
RegionLiveRegMap =
|
||||
IsLiveOut ? DAG->getRegionLiveOutMap() : DAG->getRegionLiveInMap();
|
||||
for (unsigned I = 0; I < DAG->Regions.size(); I++) {
|
||||
auto &[RegionBegin, RegionEnd] = DAG->Regions[I];
|
||||
// Skip empty regions.
|
||||
if (RegionBegin == RegionEnd)
|
||||
continue;
|
||||
MachineInstr *RegionKey =
|
||||
IsLiveOut
|
||||
? getLastMIForRegion(DAG->Regions[I].first, DAG->Regions[I].second)
|
||||
: &*DAG->Regions[I].first;
|
||||
IsLiveOut ? getLastMIForRegion(RegionBegin, RegionEnd) : &*RegionBegin;
|
||||
IdxToInstruction[I] = RegionKey;
|
||||
}
|
||||
}
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Loading…
x
Reference in New Issue
Block a user