[llvm] CodeGen: Fix double counting bundles in inst size verification (PR #191460)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 16 11:51:33 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-loongarch
Author: Matt Arsenault (arsenm)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/191460.diff
19 Files Affected:
- (modified) llvm/include/llvm/CodeGen/TargetInstrInfo.h (+8)
- (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (-7)
- (modified) llvm/lib/CodeGen/TargetInstrInfo.cpp (+12)
- (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.cpp (+1-12)
- (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.h (-2)
- (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (-12)
- (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.h (-1)
- (modified) llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp (+1-12)
- (modified) llvm/lib/Target/ARM/ARMBaseInstrInfo.h (-2)
- (modified) llvm/lib/Target/CSKY/CSKYInstrInfo.cpp (+2)
- (modified) llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp (+2)
- (modified) llvm/lib/Target/MSP430/MSP430InstrInfo.cpp (+2)
- (modified) llvm/lib/Target/Mips/MipsInstrInfo.cpp (+2)
- (modified) llvm/lib/Target/PowerPC/PPCInstrInfo.cpp (+2)
- (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+1-12)
- (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.h (-2)
- (modified) llvm/lib/Target/Sparc/SparcInstrInfo.cpp (+3)
- (modified) llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp (+2)
- (modified) llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp (+2)
``````````diff
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/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) {
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();
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/191460
More information about the llvm-commits
mailing list