[llvm-branch-commits] [llvm] AMDGPU: Fix fixme for out of bounds indexing in usesConstantBus check (PR #155603)
Matt Arsenault via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Sep 2 08:38:14 PDT 2025
https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/155603
>From a5a742a691fba41cf607a0ce20382a20d8719777 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Wed, 27 Aug 2025 16:19:23 +0900
Subject: [PATCH 1/2] 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.
---
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | 62 ++++++++++++++++----------
llvm/lib/Target/AMDGPU/SIInstrInfo.h | 4 ++
2 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index c5e8f95748cf1..4cf8fd5eb594f 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -4758,6 +4758,35 @@ MachineInstr *SIInstrInfo::buildShrunkInst(MachineInstr &MI,
return Inst32;
}
+bool SIInstrInfo::physRegUsesConstantBus(const MachineOperand &RegOp) const {
+ // Null is free
+ Register Reg = RegOp.getReg();
+ if (Reg == AMDGPU::SGPR_NULL || Reg == AMDGPU::SGPR_NULL64)
+ return false;
+
+ // SGPRs use the constant bus
+
+ // FIXME: implicit registers that are not part of the MCInstrDesc's implicit
+ // physical register operands should also count.
+ if (RegOp.isImplicit())
+ return Reg == AMDGPU::VCC || Reg == AMDGPU::VCC_LO || Reg == AMDGPU::M0;
+
+ // Normal exec read does not count.
+ if ((Reg == AMDGPU::EXEC || Reg == AMDGPU::EXEC_LO) && RegOp.isImplicit())
+ return false;
+
+ // SGPRs use the constant bus
+ return AMDGPU::SReg_32RegClass.contains(Reg) ||
+ AMDGPU::SReg_64RegClass.contains(Reg);
+}
+
+bool SIInstrInfo::regUsesConstantBus(const MachineOperand &RegOp,
+ const MachineRegisterInfo &MRI) const {
+ Register Reg = RegOp.getReg();
+ return Reg.isVirtual() ? RI.isSGPRClass(MRI.getRegClass(Reg))
+ : physRegUsesConstantBus(RegOp);
+}
+
bool SIInstrInfo::usesConstantBus(const MachineRegisterInfo &MRI,
const MachineOperand &MO,
const MCOperandInfo &OpInfo) const {
@@ -4765,23 +4794,9 @@ bool SIInstrInfo::usesConstantBus(const MachineRegisterInfo &MRI,
if (!MO.isReg())
return !isInlineConstant(MO, OpInfo);
- if (!MO.isUse())
- return false;
-
- if (MO.getReg().isVirtual())
- return RI.isSGPRClass(MRI.getRegClass(MO.getReg()));
-
- // Null is free
- if (MO.getReg() == AMDGPU::SGPR_NULL || MO.getReg() == AMDGPU::SGPR_NULL64)
- return false;
-
- // SGPRs use the constant bus
- if (MO.isImplicit()) {
- return MO.getReg() == AMDGPU::M0 || MO.getReg() == AMDGPU::VCC ||
- MO.getReg() == AMDGPU::VCC_LO;
- }
- return AMDGPU::SReg_32RegClass.contains(MO.getReg()) ||
- AMDGPU::SReg_64RegClass.contains(MO.getReg());
+ Register Reg = MO.getReg();
+ return Reg.isVirtual() ? RI.isSGPRClass(MRI.getRegClass(Reg))
+ : physRegUsesConstantBus(MO);
}
static Register findImplicitSGPRRead(const MachineInstr &MI) {
@@ -6250,13 +6265,12 @@ bool SIInstrInfo::isOperandLegal(const MachineInstr &MI, unsigned OpIdx,
continue;
const MachineOperand &Op = MI.getOperand(i);
if (Op.isReg()) {
- RegSubRegPair SGPR(Op.getReg(), Op.getSubReg());
- if (!SGPRsUsed.count(SGPR) &&
- // FIXME: This can access off the end of the operands() array.
- usesConstantBus(MRI, Op, InstDesc.operands().begin()[i])) {
- if (--ConstantBusLimit <= 0)
- return false;
- SGPRsUsed.insert(SGPR);
+ if (Op.isUse()) {
+ RegSubRegPair SGPR(Op.getReg(), Op.getSubReg());
+ if (regUsesConstantBus(Op, MRI) && SGPRsUsed.insert(SGPR).second) {
+ if (--ConstantBusLimit <= 0)
+ return false;
+ }
}
} else if (AMDGPU::isSISrcOperand(InstDesc, i) &&
!isInlineConstant(Op, InstDesc.operands()[i])) {
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.h b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
index 958af0ff1147f..2f9f5c54406a3 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
@@ -1195,6 +1195,10 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
/// This function will return false if you pass it a 32-bit instruction.
bool hasVALU32BitEncoding(unsigned Opcode) const;
+ bool physRegUsesConstantBus(const MachineOperand &Reg) const;
+ bool regUsesConstantBus(const MachineOperand &Reg,
+ const MachineRegisterInfo &MRI) const;
+
/// Returns true if this operand uses the constant bus.
bool usesConstantBus(const MachineRegisterInfo &MRI,
const MachineOperand &MO,
>From 249e2ec6420aff101ac186bda6483eb5a239fee9 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Thu, 28 Aug 2025 13:43:12 +0900
Subject: [PATCH 2/2] merge exec check
---
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 4cf8fd5eb594f..d3bda9f3875e3 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -4767,14 +4767,10 @@ bool SIInstrInfo::physRegUsesConstantBus(const MachineOperand &RegOp) const {
// SGPRs use the constant bus
// FIXME: implicit registers that are not part of the MCInstrDesc's implicit
- // physical register operands should also count.
+ // physical register operands should also count, except for exec.
if (RegOp.isImplicit())
return Reg == AMDGPU::VCC || Reg == AMDGPU::VCC_LO || Reg == AMDGPU::M0;
- // Normal exec read does not count.
- if ((Reg == AMDGPU::EXEC || Reg == AMDGPU::EXEC_LO) && RegOp.isImplicit())
- return false;
-
// SGPRs use the constant bus
return AMDGPU::SReg_32RegClass.contains(Reg) ||
AMDGPU::SReg_64RegClass.contains(Reg);
More information about the llvm-branch-commits
mailing list