CodeGen: Fix double counting bundles in inst size verification - #191460
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Hm, interesting. I did this because there were a lot of failures in backends that use instructions bundles. If this is the convention, we should probably document it in the getInstSizeInBytes() doc comment. This means that code like llvm-project/llvm/lib/Target/Mips/MipsBranchExpansion.cpp Lines 300 to 302 in d5a8af5 |
Assuming the mips implementation correctly handled bundles, it wouldn't double count. the instrs() iterator should visit the BUNDLE header, followed by the next unbundled instruction |
|
Hm, I thought it was the other way around? Default iteration skips bundled instructions but instrs() visits everything? |
Oh, yes. instrs() is instr_iterator, regular range is iterator |
faec56f to
af01d27
Compare
|
@llvm/pr-subscribers-backend-sparc @llvm/pr-subscribers-backend-mips Author: Matt Arsenault (arsenm) ChangesThe AMDGPU implementation handles bundles by summing the This loop is over the iterator, not instr_iterator, so it should Full diff: https://github.com/llvm/llvm-project/pull/191460.diff 19 Files Affected:
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();
}
|
|
@llvm/pr-subscribers-backend-loongarch Author: Matt Arsenault (arsenm) ChangesThe AMDGPU implementation handles bundles by summing the This loop is over the iterator, not instr_iterator, so it should Full diff: https://github.com/llvm/llvm-project/pull/191460.diff 19 Files Affected:
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();
}
|
|
ping |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
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.
Co-authored-by: Sam Elliott <aelliott@qti.qualcomm.com>
2155bfb to
30aa92a
Compare
…191460) 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.
…191460) 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.
) MIPS branch/jump instructions (B, BEQ, JALR64Pseudo, PseudoReturn64, etc.) have a delay slot. The actual encoded size is 8 bytes (instr + NOP). This fixes "out of range PC16 fixup" errors on large functions. This issue was exposed in llvm 23 by commit pr #191460 which changed MipsBranchExpansion to use MBB::iterator instead of instr_iterator, making the MBB size calculation more accurate and revealing the pre-existing bug. Thanks for the pr #187703 `AllowOverEstimate` to help find instr which actual size mismatch expected size . Fix #112010.
…#216665) MIPS branch/jump instructions (B, BEQ, JALR64Pseudo, PseudoReturn64, etc.) have a delay slot. The actual encoded size is 8 bytes (instr + NOP). This fixes "out of range PC16 fixup" errors on large functions. This issue was exposed in llvm 23 by commit pr llvm#191460 which changed MipsBranchExpansion to use MBB::iterator instead of instr_iterator, making the MBB size calculation more accurate and revealing the pre-existing bug. Thanks for the pr llvm#187703 `AllowOverEstimate` to help find instr which actual size mismatch expected size . Fix llvm#112010. (cherry picked from commit a97f512)
…#216665) MIPS branch/jump instructions (B, BEQ, JALR64Pseudo, PseudoReturn64, etc.) have a delay slot. The actual encoded size is 8 bytes (instr + NOP). This fixes "out of range PC16 fixup" errors on large functions. This issue was exposed in llvm 23 by commit pr llvm#191460 which changed MipsBranchExpansion to use MBB::iterator instead of instr_iterator, making the MBB size calculation more accurate and revealing the pre-existing bug. Thanks for the pr llvm#187703 `AllowOverEstimate` to help find instr which actual size mismatch expected size . Fix llvm#112010.

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.