Skip to content

Commit 0563569

Browse files
alex-tarsenm
andauthored
[AMDGPU] SIFixSgprCopies should not process twice VGPR to SGPR copies inserted by PHI preprocessing. (llvm#134153)
PHI operands and results must belong to the same register class. If a PHI node produces an SGPR, but one of its operands is a VGPR, we insert a VGPR-to-SGPR copy in the operand’s source block. The PHI operand is then updated to use the destination register of the inserted copy. These inserted copies are processed immediately when they are created. Therefore, we should avoid reprocessing them when handling their parent block later. --------- Co-authored-by: Matt Arsenault <[email protected]>
1 parent 3c6c5c7 commit 0563569

38 files changed

+8192
-7016
lines changed

llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ class SIFixSGPRCopies {
127127
unsigned NextVGPRToSGPRCopyID = 0;
128128
MapVector<unsigned, V2SCopyInfo> V2SCopies;
129129
DenseMap<MachineInstr *, SetVector<unsigned>> SiblingPenalty;
130+
DenseSet<MachineInstr *> PHISources;
130131

131132
public:
132133
MachineRegisterInfo *MRI;
@@ -691,10 +692,8 @@ bool SIFixSGPRCopies::run(MachineFunction &MF) {
691692
TII->get(AMDGPU::COPY), NewDst)
692693
.addReg(MO.getReg());
693694
MO.setReg(NewDst);
694-
695-
// FIXME: We are transitively revisiting users of this
696-
// instruction for every input.
697695
analyzeVGPRToSGPRCopy(NewCopy);
696+
PHISources.insert(NewCopy);
698697
}
699698
}
700699
}
@@ -801,6 +800,7 @@ bool SIFixSGPRCopies::run(MachineFunction &MF) {
801800
RegSequences.clear();
802801
PHINodes.clear();
803802
S2VCopies.clear();
803+
PHISources.clear();
804804

805805
return true;
806806
}
@@ -926,13 +926,13 @@ bool SIFixSGPRCopies::lowerSpecialCase(MachineInstr &MI,
926926
}
927927

928928
void SIFixSGPRCopies::analyzeVGPRToSGPRCopy(MachineInstr* MI) {
929+
if (PHISources.contains(MI))
930+
return;
929931
Register DstReg = MI->getOperand(0).getReg();
930932
const TargetRegisterClass *DstRC = MRI->getRegClass(DstReg);
931933

932934
V2SCopyInfo Info(getNextVGPRToSGPRCopyId(), MI,
933935
TRI->getRegSizeInBits(*DstRC));
934-
V2SCopies[Info.ID] = Info;
935-
936936
SmallVector<MachineInstr *, 8> AnalysisWorklist;
937937
// Needed because the SSA is not a tree but a graph and may have
938938
// forks and joins. We should not then go same way twice.
@@ -971,16 +971,18 @@ void SIFixSGPRCopies::analyzeVGPRToSGPRCopy(MachineInstr* MI) {
971971
}
972972
} else if (Inst->getNumExplicitDefs() != 0) {
973973
Register Reg = Inst->getOperand(0).getReg();
974-
if (TRI->isSGPRReg(*MRI, Reg) && !TII->isVALU(*Inst))
974+
if (Reg.isVirtual() && TRI->isSGPRReg(*MRI, Reg) && !TII->isVALU(*Inst)) {
975975
for (auto &U : MRI->use_instructions(Reg))
976976
Users.push_back(&U);
977+
}
977978
}
978979
for (auto *U : Users) {
979980
if (TII->isSALU(*U))
980981
Info.SChain.insert(U);
981982
AnalysisWorklist.push_back(U);
982983
}
983984
}
985+
V2SCopies[Info.ID] = Info;
984986
}
985987

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

0 commit comments

Comments
 (0)