Skip to content

Commit 07fefca

Browse files
committed
AMDGPU: Fix fixme for out of bounds indexing in usesConstantBus check
This loop over all the operands in the MachineInstr will eventually go past the end of the MCInstrDesc's explicit operands. We don't need the instr desc to compute the constant bus usage, just the register and whether it's implicit or not. The check here is slightly conservative. e.g. a random vcc implicit use appended to an instruction will falsely report a constant bus use.
1 parent 2a49ebf commit 07fefca

File tree

2 files changed

+42
-24
lines changed

2 files changed

+42
-24
lines changed

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4758,30 +4758,45 @@ MachineInstr *SIInstrInfo::buildShrunkInst(MachineInstr &MI,
47584758
return Inst32;
47594759
}
47604760

4761+
bool SIInstrInfo::physRegUsesConstantBus(const MachineOperand &RegOp) const {
4762+
// Null is free
4763+
Register Reg = RegOp.getReg();
4764+
if (Reg == AMDGPU::SGPR_NULL || Reg == AMDGPU::SGPR_NULL64)
4765+
return false;
4766+
4767+
// SGPRs use the constant bus
4768+
4769+
// FIXME: implicit registers that are not part of the MCInstrDesc's implicit
4770+
// physical register operands should also count.
4771+
if (RegOp.isImplicit())
4772+
return Reg == AMDGPU::VCC || Reg == AMDGPU::VCC_LO || Reg == AMDGPU::M0;
4773+
4774+
// Normal exec read does not count.
4775+
if ((Reg == AMDGPU::EXEC || Reg == AMDGPU::EXEC_LO) && RegOp.isImplicit())
4776+
return false;
4777+
4778+
// SGPRs use the constant bus
4779+
return AMDGPU::SReg_32RegClass.contains(Reg) ||
4780+
AMDGPU::SReg_64RegClass.contains(Reg);
4781+
}
4782+
4783+
bool SIInstrInfo::regUsesConstantBus(const MachineOperand &RegOp,
4784+
const MachineRegisterInfo &MRI) const {
4785+
Register Reg = RegOp.getReg();
4786+
return Reg.isVirtual() ? RI.isSGPRClass(MRI.getRegClass(Reg))
4787+
: physRegUsesConstantBus(RegOp);
4788+
}
4789+
47614790
bool SIInstrInfo::usesConstantBus(const MachineRegisterInfo &MRI,
47624791
const MachineOperand &MO,
47634792
const MCOperandInfo &OpInfo) const {
47644793
// Literal constants use the constant bus.
47654794
if (!MO.isReg())
47664795
return !isInlineConstant(MO, OpInfo);
47674796

4768-
if (!MO.isUse())
4769-
return false;
4770-
4771-
if (MO.getReg().isVirtual())
4772-
return RI.isSGPRClass(MRI.getRegClass(MO.getReg()));
4773-
4774-
// Null is free
4775-
if (MO.getReg() == AMDGPU::SGPR_NULL || MO.getReg() == AMDGPU::SGPR_NULL64)
4776-
return false;
4777-
4778-
// SGPRs use the constant bus
4779-
if (MO.isImplicit()) {
4780-
return MO.getReg() == AMDGPU::M0 || MO.getReg() == AMDGPU::VCC ||
4781-
MO.getReg() == AMDGPU::VCC_LO;
4782-
}
4783-
return AMDGPU::SReg_32RegClass.contains(MO.getReg()) ||
4784-
AMDGPU::SReg_64RegClass.contains(MO.getReg());
4797+
Register Reg = MO.getReg();
4798+
return Reg.isVirtual() ? RI.isSGPRClass(MRI.getRegClass(Reg))
4799+
: physRegUsesConstantBus(MO);
47854800
}
47864801

47874802
static Register findImplicitSGPRRead(const MachineInstr &MI) {
@@ -6250,13 +6265,12 @@ bool SIInstrInfo::isOperandLegal(const MachineInstr &MI, unsigned OpIdx,
62506265
continue;
62516266
const MachineOperand &Op = MI.getOperand(i);
62526267
if (Op.isReg()) {
6253-
RegSubRegPair SGPR(Op.getReg(), Op.getSubReg());
6254-
if (!SGPRsUsed.count(SGPR) &&
6255-
// FIXME: This can access off the end of the operands() array.
6256-
usesConstantBus(MRI, Op, InstDesc.operands().begin()[i])) {
6257-
if (--ConstantBusLimit <= 0)
6258-
return false;
6259-
SGPRsUsed.insert(SGPR);
6268+
if (Op.isUse()) {
6269+
RegSubRegPair SGPR(Op.getReg(), Op.getSubReg());
6270+
if (regUsesConstantBus(Op, MRI) && SGPRsUsed.insert(SGPR).second) {
6271+
if (--ConstantBusLimit <= 0)
6272+
return false;
6273+
}
62606274
}
62616275
} else if (AMDGPU::isSISrcOperand(InstDesc, i) &&
62626276
!isInlineConstant(Op, InstDesc.operands()[i])) {

llvm/lib/Target/AMDGPU/SIInstrInfo.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,6 +1195,10 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
11951195
/// This function will return false if you pass it a 32-bit instruction.
11961196
bool hasVALU32BitEncoding(unsigned Opcode) const;
11971197

1198+
bool physRegUsesConstantBus(const MachineOperand &Reg) const;
1199+
bool regUsesConstantBus(const MachineOperand &Reg,
1200+
const MachineRegisterInfo &MRI) const;
1201+
11981202
/// Returns true if this operand uses the constant bus.
11991203
bool usesConstantBus(const MachineRegisterInfo &MRI,
12001204
const MachineOperand &MO,

0 commit comments

Comments
 (0)