https://github.com/arsenm updated
https://github.com/llvm/llvm-project/pull/155603
>From 5c284f46a1063d5d0788c25a0d37ba019c171d54 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <matthew.arsena...@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 a2bb311c88ec93f730e7707310d491edb02de537 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <matthew.arsena...@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);
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
