[llvm] CodeGen: Fix double counting bundles in inst size verification (PR #191460)
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 17 03:53:40 PDT 2026
https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/191460
>From f666cf5e00191cfb5aaa1b7610a9fcbf79aabbf3 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Fri, 10 Apr 2026 17:44:53 +0200
Subject: [PATCH 1/3] CodeGen: Fix double counting bundles in inst size
verification
The AMDGPU implementation handles bundles by summing the
member instructions. This was starting with the size of the
bundle instruction, then re-adding all of the same instructions.
This loop is over the iterator, not instr_iterator, so it should
not be looking through the bundled instructions. Most of the other
uses of getInstSizeInBytes are also on the iterator, not the
instr_iterator so the convention seems to be targets need to handle
BUNDLE correctly themselves.
---
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 7 -------
1 file changed, 7 deletions(-)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 2225c24fcd7be..8c75ca7c17f7e 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -2289,13 +2289,6 @@ void AsmPrinter::emitFunctionBody() {
TII->getInstSizeVerifyMode(MI);
if (Mode != TargetInstrInfo::InstSizeVerifyMode::NoVerify) {
unsigned ExpectedSize = TII->getInstSizeInBytes(MI);
- if (MI.isBundled()) {
- // Bundled instructions are emitted together.
- auto It = MI.getIterator(), End = MBB.instr_end();
- for (++It; It != End && It->isInsideBundle(); ++It)
- ExpectedSize += TII->getInstSizeInBytes(*It);
- }
-
MCFragment *NewFragment = OutStreamer->getCurrentFragment();
unsigned ActualSize;
if (OldFragment == NewFragment) {
>From af01d274c50182098f4f27025602953faeffbf03 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Thu, 16 Apr 2026 19:19:44 +0100
Subject: [PATCH 2/3] Bundle size in all targets
---
llvm/include/llvm/CodeGen/TargetInstrInfo.h | 8 ++++++++
llvm/lib/CodeGen/TargetInstrInfo.cpp | 12 ++++++++++++
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 13 +------------
llvm/lib/Target/AArch64/AArch64InstrInfo.h | 2 --
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | 12 ------------
llvm/lib/Target/AMDGPU/SIInstrInfo.h | 1 -
llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp | 13 +------------
llvm/lib/Target/ARM/ARMBaseInstrInfo.h | 2 --
llvm/lib/Target/CSKY/CSKYInstrInfo.cpp | 2 ++
llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp | 2 ++
llvm/lib/Target/MSP430/MSP430InstrInfo.cpp | 2 ++
llvm/lib/Target/Mips/MipsInstrInfo.cpp | 2 ++
llvm/lib/Target/PowerPC/PPCInstrInfo.cpp | 2 ++
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 13 +------------
llvm/lib/Target/RISCV/RISCVInstrInfo.h | 2 --
llvm/lib/Target/Sparc/SparcInstrInfo.cpp | 3 +++
llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp | 2 ++
llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp | 2 ++
18 files changed, 40 insertions(+), 55 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 49562f16bf371..f895fc199f6d6 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -422,8 +422,16 @@ class LLVM_ABI TargetInstrInfo : public MCInstrInfo {
return MI->isTerminator() && isUnspillableTerminatorImpl(MI);
}
+ /// Sum the sizes of instructions inside of a BUNDLE, by calling
+ /// getInstBundleSize on each. This is a utility function for implementations
+ /// of getInstSizeInBytes to use.
+ unsigned getInstBundleSize(const MachineInstr &MI) const;
+
/// Returns the size in bytes of the specified MachineInstr, or ~0U
/// when this function is not implemented by a target.
+
+ /// For BUNDLE instructions, target implementations are responsible for
+ /// accounting for the size of all bundled instructions.
virtual unsigned getInstSizeInBytes(const MachineInstr &MI) const {
return ~0U;
}
diff --git a/llvm/lib/CodeGen/TargetInstrInfo.cpp b/llvm/lib/CodeGen/TargetInstrInfo.cpp
index ba836df02048c..17fd2607d2094 100644
--- a/llvm/lib/CodeGen/TargetInstrInfo.cpp
+++ b/llvm/lib/CodeGen/TargetInstrInfo.cpp
@@ -148,6 +148,18 @@ unsigned TargetInstrInfo::getInlineAsmLength(
return Length;
}
+unsigned TargetInstrInfo::getInstBundleSize(const MachineInstr &MI) const {
+ unsigned Size = 0;
+ MachineBasicBlock::const_instr_iterator I = MI.getIterator();
+ MachineBasicBlock::const_instr_iterator E = MI.getParent()->instr_end();
+ while (++I != E && I->isInsideBundle()) {
+ assert(!I->isBundle() && "No nested bundle!");
+ Size += getInstSizeInBytes(*I);
+ }
+
+ return Size;
+}
+
/// ReplaceTailWithBranchTo - Delete the instruction OldInst and everything
/// after it, replacing it with an unconditional branch to NewDest.
void
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 9d805dad07c1c..8d158b6b86d16 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -220,24 +220,13 @@ unsigned AArch64InstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
NumBytes = MI.getOperand(1).getImm();
break;
case TargetOpcode::BUNDLE:
- NumBytes = getInstBundleLength(MI);
+ NumBytes = getInstBundleSize(MI);
break;
}
return NumBytes;
}
-unsigned AArch64InstrInfo::getInstBundleLength(const MachineInstr &MI) const {
- unsigned Size = 0;
- MachineBasicBlock::const_instr_iterator I = MI.getIterator();
- MachineBasicBlock::const_instr_iterator E = MI.getParent()->instr_end();
- while (++I != E && I->isInsideBundle()) {
- assert(!I->isBundle() && "No nested bundle!");
- Size += getInstSizeInBytes(*I);
- }
- return Size;
-}
-
static void parseCondBranch(MachineInstr *LastInst, MachineBasicBlock *&Target,
SmallVectorImpl<MachineOperand> &Cond) {
// Block ends with fall-through condbranch.
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
index 52bd8e3f9580c..0c4d4e65644a3 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
@@ -599,8 +599,6 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {
isCopyLikeInstrImpl(const MachineInstr &MI) const override;
private:
- unsigned getInstBundleLength(const MachineInstr &MI) const;
-
/// Sets the offsets on outlined instructions in \p MBB which use SP
/// so that they will be valid post-outlining.
///
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index bc3052b139d18..fa5492955a1d7 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -9799,18 +9799,6 @@ Register SIInstrInfo::isStoreToStackSlot(const MachineInstr &MI,
return Register();
}
-unsigned SIInstrInfo::getInstBundleSize(const MachineInstr &MI) const {
- unsigned Size = 0;
- MachineBasicBlock::const_instr_iterator I = MI.getIterator();
- MachineBasicBlock::const_instr_iterator E = MI.getParent()->instr_end();
- while (++I != E && I->isInsideBundle()) {
- assert(!I->isBundle() && "No nested bundle!");
- Size += getInstSizeInBytes(*I);
- }
-
- return Size;
-}
-
unsigned SIInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
unsigned Opc = MI.getOpcode();
const MCInstrDesc &Desc = getMCOpcodeFromPseudo(Opc);
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.h b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
index 3c1232ac098a0..c775cb5c8876e 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
@@ -1579,7 +1579,6 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
Register isStoreToStackSlot(const MachineInstr &MI, int &FrameIndex,
TypeSize &MemBytes) const override;
- unsigned getInstBundleSize(const MachineInstr &MI) const;
unsigned getInstSizeInBytes(const MachineInstr &MI) const override;
bool mayAccessFlatAddressSpace(const MachineInstr &MI) const;
diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
index d77bf84cde5e1..d121c3160b24e 100644
--- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -618,7 +618,7 @@ unsigned ARMBaseInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
// example.
return MCID.getSize();
case TargetOpcode::BUNDLE:
- return getInstBundleLength(MI);
+ return getInstBundleSize(MI);
case TargetOpcode::COPY:
if (!MF->getInfo<ARMFunctionInfo>()->isThumbFunction())
return 4;
@@ -645,17 +645,6 @@ unsigned ARMBaseInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
}
}
-unsigned ARMBaseInstrInfo::getInstBundleLength(const MachineInstr &MI) const {
- unsigned Size = 0;
- MachineBasicBlock::const_instr_iterator I = MI.getIterator();
- MachineBasicBlock::const_instr_iterator E = MI.getParent()->instr_end();
- while (++I != E && I->isInsideBundle()) {
- assert(!I->isBundle() && "No nested bundle!");
- Size += getInstSizeInBytes(*I);
- }
- return Size;
-}
-
void ARMBaseInstrInfo::copyFromCPSR(MachineBasicBlock &MBB,
MachineBasicBlock::iterator I,
MCRegister DestReg, bool KillSrc,
diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.h b/llvm/lib/Target/ARM/ARMBaseInstrInfo.h
index 5aee9e648fbd0..94595ab2b338b 100644
--- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.h
+++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.h
@@ -416,8 +416,6 @@ class ARMBaseInstrInfo : public ARMGenInstrInfo {
bool checkAndUpdateStackOffset(MachineInstr *MI, int64_t Fixup,
bool Updt) const;
- unsigned getInstBundleLength(const MachineInstr &MI) const;
-
std::optional<unsigned> getVLDMDefCycle(const InstrItineraryData *ItinData,
const MCInstrDesc &DefMCID,
unsigned DefClass, unsigned DefIdx,
diff --git a/llvm/lib/Target/CSKY/CSKYInstrInfo.cpp b/llvm/lib/Target/CSKY/CSKYInstrInfo.cpp
index 904cdc7e2766e..3a28b383c194a 100644
--- a/llvm/lib/Target/CSKY/CSKYInstrInfo.cpp
+++ b/llvm/lib/Target/CSKY/CSKYInstrInfo.cpp
@@ -617,6 +617,8 @@ unsigned CSKYInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
const MachineFunction *MF = MI.getParent()->getParent();
const char *AsmStr = MI.getOperand(0).getSymbolName();
return getInlineAsmLength(AsmStr, *MF->getTarget().getMCAsmInfo());
+ case TargetOpcode::BUNDLE:
+ return getInstBundleSize(MI);
}
}
}
diff --git a/llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp b/llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp
index 4b5ce311f7d52..631565b963b79 100644
--- a/llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp
+++ b/llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp
@@ -280,6 +280,8 @@ unsigned LoongArchInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
case TargetOpcode::PATCHABLE_TAIL_CALL:
// Size of xray sled (branch + 11 nops).
return 12 * 4;
+ case TargetOpcode::BUNDLE:
+ return getInstBundleSize(MI);
}
return NumBytes;
}
diff --git a/llvm/lib/Target/MSP430/MSP430InstrInfo.cpp b/llvm/lib/Target/MSP430/MSP430InstrInfo.cpp
index 14a863b2407db..61ecb3e13669d 100644
--- a/llvm/lib/Target/MSP430/MSP430InstrInfo.cpp
+++ b/llvm/lib/Target/MSP430/MSP430InstrInfo.cpp
@@ -300,6 +300,8 @@ unsigned MSP430InstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
return TII.getInlineAsmLength(MI.getOperand(0).getSymbolName(),
*MF->getTarget().getMCAsmInfo());
}
+ case TargetOpcode::BUNDLE:
+ return getInstBundleSize(MI);
}
return Desc.getSize();
diff --git a/llvm/lib/Target/Mips/MipsInstrInfo.cpp b/llvm/lib/Target/Mips/MipsInstrInfo.cpp
index c08c963a33c71..48e71c93f7881 100644
--- a/llvm/lib/Target/Mips/MipsInstrInfo.cpp
+++ b/llvm/lib/Target/Mips/MipsInstrInfo.cpp
@@ -714,6 +714,8 @@ unsigned MipsInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
const char *AsmStr = MI.getOperand(0).getSymbolName();
return getInlineAsmLength(AsmStr, *MF->getTarget().getMCAsmInfo());
}
+ case TargetOpcode::BUNDLE:
+ return getInstBundleSize(MI);
case TargetOpcode::PATCHABLE_FUNCTION_ENTER:
case TargetOpcode::PATCHABLE_FUNCTION_EXIT:
case TargetOpcode::PATCHABLE_TAIL_CALL:
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
index d458c0005f99b..f854dca003964 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -3039,6 +3039,8 @@ unsigned PPCInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
bool IsConditional = RetOpcode == PPC::BCCLR;
return (8 + IsConditional) * 4;
}
+ case TargetOpcode::BUNDLE:
+ return getInstBundleSize(MI);
default:
return get(Opcode).getSize();
}
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 23bbaced94a5e..72c2b8b8ee35c 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -1990,7 +1990,7 @@ unsigned RISCVInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
}
if (Opcode == TargetOpcode::BUNDLE)
- return getInstBundleLength(MI);
+ return getInstBundleSize(MI);
if (MI.getParent() && MI.getParent()->getParent()) {
if (isCompressibleInst(MI, STI))
@@ -2098,17 +2098,6 @@ unsigned RISCVInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
}
}
-unsigned RISCVInstrInfo::getInstBundleLength(const MachineInstr &MI) const {
- unsigned Size = 0;
- MachineBasicBlock::const_instr_iterator I = MI.getIterator();
- MachineBasicBlock::const_instr_iterator E = MI.getParent()->instr_end();
- while (++I != E && I->isInsideBundle()) {
- assert(!I->isBundle() && "No nested bundle!");
- Size += getInstSizeInBytes(*I);
- }
- return Size;
-}
-
bool RISCVInstrInfo::isAsCheapAsAMove(const MachineInstr &MI) const {
const unsigned Opcode = MI.getOpcode();
switch (Opcode) {
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index 119b2a7eae6bf..adb2fb526f26b 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -362,8 +362,6 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
const RISCVSubtarget &STI;
private:
- unsigned getInstBundleLength(const MachineInstr &MI) const;
-
bool isVectorAssociativeAndCommutative(const MachineInstr &MI,
bool Invert = false) const;
bool areRVVInstsReassociable(const MachineInstr &MI1,
diff --git a/llvm/lib/Target/Sparc/SparcInstrInfo.cpp b/llvm/lib/Target/Sparc/SparcInstrInfo.cpp
index 4d21a753c1e36..d6d7a238d336e 100644
--- a/llvm/lib/Target/Sparc/SparcInstrInfo.cpp
+++ b/llvm/lib/Target/Sparc/SparcInstrInfo.cpp
@@ -665,6 +665,9 @@ unsigned SparcInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
return getInlineAsmLength(AsmStr, *MF->getTarget().getMCAsmInfo());
}
+ if (Opcode == TargetOpcode::BUNDLE)
+ return getInstBundleSize(MI);
+
if (MI.getOpcode() == SP::GETPCX) {
const TargetMachine &TM = MI.getParent()->getParent()->getTarget();
if (TM.isPositionIndependent())
diff --git a/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp b/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp
index 5d85a64844592..19db134a7fd8d 100644
--- a/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp
@@ -1831,6 +1831,8 @@ unsigned SystemZInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
return 18;
if (MI.getOpcode() == TargetOpcode::PATCHABLE_RET)
return 18 + (MI.getOperand(0).getImm() == SystemZ::CondReturn ? 4 : 0);
+ if (MI.getOpcode() == TargetOpcode::BUNDLE)
+ return getInstBundleSize(MI);
return MI.getDesc().getSize();
}
diff --git a/llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp b/llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp
index 1eb42d1cd0c5d..36ffb921d7c91 100644
--- a/llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp
+++ b/llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp
@@ -223,6 +223,8 @@ unsigned XtensaInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
const char *AsmStr = MI.getOperand(0).getSymbolName();
return getInlineAsmLength(AsmStr, *MF->getTarget().getMCAsmInfo());
}
+ case TargetOpcode::BUNDLE:
+ return getInstBundleSize(MI);
default:
return MI.getDesc().getSize();
}
>From bb3b9f97f30f70d299afabd6db14b55ead83e067 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Fri, 17 Apr 2026 11:53:07 +0100
Subject: [PATCH 3/3] Fix wrong mips iterator
---
llvm/lib/Target/Mips/MipsBranchExpansion.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/Mips/MipsBranchExpansion.cpp b/llvm/lib/Target/Mips/MipsBranchExpansion.cpp
index 3720c936643b4..7fcacb36eacd9 100644
--- a/llvm/lib/Target/Mips/MipsBranchExpansion.cpp
+++ b/llvm/lib/Target/Mips/MipsBranchExpansion.cpp
@@ -298,7 +298,7 @@ void MipsBranchExpansion::initMBBInfo() {
MachineBasicBlock *MBB = MFp->getBlockNumbered(I);
// Compute size of MBB.
- for (MachineInstr &MI : MBB->instrs())
+ for (MachineInstr &MI : *MBB)
MBBInfos[I].Size += TII->getInstSizeInBytes(MI);
}
}
More information about the llvm-commits
mailing list