Skip to content

Commit dea5aa7

Browse files
authored
AMDGPU: Move insertion into V2SCopies map (#130776)
Insert the start instruction directly into the map before the uses. This prevents improperly re-visting sgpr->vgpr phi inputs multiple times which would trigger a use after free. I don't particularly trust the iteration scheme here. This is also unnecessarily revisting transitive users of a phi or reg_sequence for every input operand, but I will address that separately. Fixes #130646. I also believe it fixes #130119, although that test fails less consistently for me.
1 parent 6542cf1 commit dea5aa7

39 files changed

+7431
-8128
lines changed

llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp

+5-1
Original file line numberDiff line numberDiff line change
@@ -691,6 +691,9 @@ bool SIFixSGPRCopies::run(MachineFunction &MF) {
691691
TII->get(AMDGPU::COPY), NewDst)
692692
.addReg(MO.getReg());
693693
MO.setReg(NewDst);
694+
695+
// FIXME: We are transitively revisiting users of this
696+
// instruction for every input.
694697
analyzeVGPRToSGPRCopy(NewCopy);
695698
}
696699
}
@@ -928,6 +931,8 @@ void SIFixSGPRCopies::analyzeVGPRToSGPRCopy(MachineInstr* MI) {
928931

929932
V2SCopyInfo Info(getNextVGPRToSGPRCopyId(), MI,
930933
TRI->getRegSizeInBits(*DstRC));
934+
V2SCopies[Info.ID] = Info;
935+
931936
SmallVector<MachineInstr *, 8> AnalysisWorklist;
932937
// Needed because the SSA is not a tree but a graph and may have
933938
// forks and joins. We should not then go same way twice.
@@ -976,7 +981,6 @@ void SIFixSGPRCopies::analyzeVGPRToSGPRCopy(MachineInstr* MI) {
976981
AnalysisWorklist.push_back(U);
977982
}
978983
}
979-
V2SCopies[Info.ID] = Info;
980984
}
981985

982986
// The main function that computes the VGPR to SGPR copy score

llvm/test/CodeGen/AMDGPU/agpr-copy-no-free-registers.ll

+231-233
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)