Skip to content

Commit 16c1de0

Browse files
committed
[AMDGPU] Verify dominance when rewriting spills to registers
When performing spill elimination in the AGPR copy rewrite pass it was possible to see spill reloads that were not dominated by any store. This caused invalid MIR to be generated where vreg uses were not dominated by defs. This patch adds a dominance check before rewriting spills.
1 parent a3b5b4b commit 16c1de0

File tree

2 files changed

+2146
-4
lines changed

2 files changed

+2146
-4
lines changed

llvm/lib/Target/AMDGPU/AMDGPURewriteAGPRCopyMFMA.cpp

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "llvm/CodeGen/LiveIntervals.h"
3131
#include "llvm/CodeGen/LiveRegMatrix.h"
3232
#include "llvm/CodeGen/LiveStacks.h"
33+
#include "llvm/CodeGen/MachineDominators.h"
3334
#include "llvm/CodeGen/MachineFrameInfo.h"
3435
#include "llvm/CodeGen/MachineFunctionPass.h"
3536
#include "llvm/CodeGen/VirtRegMap.h"
@@ -58,6 +59,7 @@ class AMDGPURewriteAGPRCopyMFMAImpl {
5859
LiveIntervals &LIS;
5960
LiveStacks &LSS;
6061
const RegisterClassInfo &RegClassInfo;
62+
MachineDominatorTree &MDT;
6163

6264
bool attemptReassignmentsToAGPR(SmallSetVector<Register, 4> &InterferingRegs,
6365
MCPhysReg PrefPhysReg) const;
@@ -66,10 +68,11 @@ class AMDGPURewriteAGPRCopyMFMAImpl {
6668
AMDGPURewriteAGPRCopyMFMAImpl(MachineFunction &MF, VirtRegMap &VRM,
6769
LiveRegMatrix &LRM, LiveIntervals &LIS,
6870
LiveStacks &LSS,
69-
const RegisterClassInfo &RegClassInfo)
71+
const RegisterClassInfo &RegClassInfo,
72+
MachineDominatorTree &MDT)
7073
: MF(MF), ST(MF.getSubtarget<GCNSubtarget>()), TII(*ST.getInstrInfo()),
7174
TRI(*ST.getRegisterInfo()), MRI(MF.getRegInfo()), VRM(VRM), LRM(LRM),
72-
LIS(LIS), LSS(LSS), RegClassInfo(RegClassInfo) {}
75+
LIS(LIS), LSS(LSS), RegClassInfo(RegClassInfo), MDT(MDT) {}
7376

7477
bool isRewriteCandidate(const MachineInstr &MI) const {
7578
return TII.isMAI(MI) && AMDGPU::getMFMASrcCVDstAGPROp(MI.getOpcode()) != -1;
@@ -515,6 +518,56 @@ void AMDGPURewriteAGPRCopyMFMAImpl::eliminateSpillsOfReassignedVGPRs() const {
515518
if (SpillReferences == SpillSlotReferences.end())
516519
continue;
517520

521+
// Verify that each spill restore is dominated by at least one spill save.
522+
SmallVector<MachineInstr *, 4> Stores, Loads;
523+
Stores.reserve(SpillReferences->second.size());
524+
Loads.reserve(SpillReferences->second.size());
525+
for (MachineInstr *MI : SpillReferences->second) {
526+
if (MI->mayStore())
527+
Stores.push_back(MI);
528+
else if (MI->mayLoad())
529+
Loads.push_back(MI);
530+
}
531+
532+
SmallVector<MachineInstr *, 4> ReachableStores;
533+
ReachableStores.reserve(Stores.size());
534+
for (MachineInstr *S : Stores)
535+
if (MDT.isReachableFromEntry(S->getParent()))
536+
ReachableStores.push_back(S);
537+
538+
if (ReachableStores.empty()) {
539+
LLVM_DEBUG(dbgs() << "Skipping " << printReg(Slot, &TRI)
540+
<< ": no reachable stores\n");
541+
continue;
542+
}
543+
544+
auto IsDominatedByAnyStore = [&](MachineInstr *LoadMI) -> bool {
545+
MachineBasicBlock *LoadMBB = LoadMI->getParent();
546+
if (!MDT.isReachableFromEntry(LoadMBB))
547+
return true;
548+
for (MachineInstr *StoreMI : ReachableStores) {
549+
MachineBasicBlock *StoreMBB = StoreMI->getParent();
550+
if (StoreMBB == LoadMBB) {
551+
for (MachineBasicBlock::iterator I = StoreMI->getIterator(),
552+
E = StoreMBB->end();
553+
I != E; ++I)
554+
if (&*I == LoadMI)
555+
return true;
556+
continue;
557+
}
558+
if (MDT.dominates(StoreMBB, LoadMBB))
559+
return true;
560+
}
561+
return false;
562+
};
563+
564+
if (!llvm::all_of(Loads, IsDominatedByAnyStore)) {
565+
LLVM_DEBUG(
566+
dbgs() << "Skipping " << printReg(Slot, &TRI)
567+
<< ": some reachable load not dominated by any store\n");
568+
continue;
569+
}
570+
518571
const TargetRegisterClass *RC = LSS.getIntervalRegClass(Slot);
519572

520573
LLVM_DEBUG(dbgs() << "Trying to eliminate " << printReg(Slot, &TRI)
@@ -603,11 +656,13 @@ class AMDGPURewriteAGPRCopyMFMALegacy : public MachineFunctionPass {
603656
AU.addRequired<VirtRegMapWrapperLegacy>();
604657
AU.addRequired<LiveRegMatrixWrapperLegacy>();
605658
AU.addRequired<LiveStacksWrapperLegacy>();
659+
AU.addRequired<MachineDominatorTreeWrapperPass>();
606660

607661
AU.addPreserved<LiveIntervalsWrapperPass>();
608662
AU.addPreserved<VirtRegMapWrapperLegacy>();
609663
AU.addPreserved<LiveRegMatrixWrapperLegacy>();
610664
AU.addPreserved<LiveStacksWrapperLegacy>();
665+
AU.addPreserved<MachineDominatorTreeWrapperPass>();
611666

612667
AU.setPreservesAll();
613668
MachineFunctionPass::getAnalysisUsage(AU);
@@ -622,6 +677,7 @@ INITIALIZE_PASS_DEPENDENCY(LiveIntervalsWrapperPass)
622677
INITIALIZE_PASS_DEPENDENCY(VirtRegMapWrapperLegacy)
623678
INITIALIZE_PASS_DEPENDENCY(LiveRegMatrixWrapperLegacy)
624679
INITIALIZE_PASS_DEPENDENCY(LiveStacksWrapperLegacy)
680+
INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
625681
INITIALIZE_PASS_END(AMDGPURewriteAGPRCopyMFMALegacy, DEBUG_TYPE,
626682
"AMDGPU Rewrite AGPR-Copy-MFMA", false, false)
627683

@@ -641,7 +697,8 @@ bool AMDGPURewriteAGPRCopyMFMALegacy::runOnMachineFunction(
641697
auto &LRM = getAnalysis<LiveRegMatrixWrapperLegacy>().getLRM();
642698
auto &LIS = getAnalysis<LiveIntervalsWrapperPass>().getLIS();
643699
auto &LSS = getAnalysis<LiveStacksWrapperLegacy>().getLS();
644-
AMDGPURewriteAGPRCopyMFMAImpl Impl(MF, VRM, LRM, LIS, LSS, RegClassInfo);
700+
auto &MDT = getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
701+
AMDGPURewriteAGPRCopyMFMAImpl Impl(MF, VRM, LRM, LIS, LSS, RegClassInfo, MDT);
645702
return Impl.run(MF);
646703
}
647704

@@ -652,10 +709,11 @@ AMDGPURewriteAGPRCopyMFMAPass::run(MachineFunction &MF,
652709
LiveRegMatrix &LRM = MFAM.getResult<LiveRegMatrixAnalysis>(MF);
653710
LiveIntervals &LIS = MFAM.getResult<LiveIntervalsAnalysis>(MF);
654711
LiveStacks &LSS = MFAM.getResult<LiveStacksAnalysis>(MF);
712+
MachineDominatorTree &MDT = MFAM.getResult<MachineDominatorTreeAnalysis>(MF);
655713
RegisterClassInfo RegClassInfo;
656714
RegClassInfo.runOnMachineFunction(MF);
657715

658-
AMDGPURewriteAGPRCopyMFMAImpl Impl(MF, VRM, LRM, LIS, LSS, RegClassInfo);
716+
AMDGPURewriteAGPRCopyMFMAImpl Impl(MF, VRM, LRM, LIS, LSS, RegClassInfo, MDT);
659717
if (!Impl.run(MF))
660718
return PreservedAnalyses::all();
661719
auto PA = getMachineFunctionPassPreservedAnalyses();

0 commit comments

Comments
 (0)