Skip to content

Commit d6a72cb

Browse files
authored
AMDGPU: Fix fixme for out of bounds indexing in usesConstantBus check (#155603)
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 1cc84bc commit d6a72cb

File tree

2 files changed

+38
-24
lines changed

2 files changed

+38
-24
lines changed

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4758,30 +4758,41 @@ 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, except for exec.
4771+
if (RegOp.isImplicit())
4772+
return Reg == AMDGPU::VCC || Reg == AMDGPU::VCC_LO || Reg == AMDGPU::M0;
4773+
4774+
// SGPRs use the constant bus
4775+
return AMDGPU::SReg_32RegClass.contains(Reg) ||
4776+
AMDGPU::SReg_64RegClass.contains(Reg);
4777+
}
4778+
4779+
bool SIInstrInfo::regUsesConstantBus(const MachineOperand &RegOp,
4780+
const MachineRegisterInfo &MRI) const {
4781+
Register Reg = RegOp.getReg();
4782+
return Reg.isVirtual() ? RI.isSGPRClass(MRI.getRegClass(Reg))
4783+
: physRegUsesConstantBus(RegOp);
4784+
}
4785+
47614786
bool SIInstrInfo::usesConstantBus(const MachineRegisterInfo &MRI,
47624787
const MachineOperand &MO,
47634788
const MCOperandInfo &OpInfo) const {
47644789
// Literal constants use the constant bus.
47654790
if (!MO.isReg())
47664791
return !isInlineConstant(MO, OpInfo);
47674792

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());
4793+
Register Reg = MO.getReg();
4794+
return Reg.isVirtual() ? RI.isSGPRClass(MRI.getRegClass(Reg))
4795+
: physRegUsesConstantBus(MO);
47854796
}
47864797

47874798
static Register findImplicitSGPRRead(const MachineInstr &MI) {
@@ -6250,13 +6261,12 @@ bool SIInstrInfo::isOperandLegal(const MachineInstr &MI, unsigned OpIdx,
62506261
continue;
62516262
const MachineOperand &Op = MI.getOperand(i);
62526263
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);
6264+
if (Op.isUse()) {
6265+
RegSubRegPair SGPR(Op.getReg(), Op.getSubReg());
6266+
if (regUsesConstantBus(Op, MRI) && SGPRsUsed.insert(SGPR).second) {
6267+
if (--ConstantBusLimit <= 0)
6268+
return false;
6269+
}
62606270
}
62616271
} else if (AMDGPU::isSISrcOperand(InstDesc, i) &&
62626272
!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)