[llvm-branch-commits] [llvm] [ARM][Thumb1] Verify EstimateFunctionSizeInBytes every time (PR #203319)
Simon Tatham via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Jun 11 09:05:55 PDT 2026
https://github.com/statham-arm created https://github.com/llvm/llvm-project/pull/203319
EstimateFunctionSizeInBytes is supposed to err on the side of overestimating the function size, but historically, has sometimes underestimated, leading to a code generation failure in ARMConstantIslandPass in the rare case where the underestimate led frame setup to fail to stack LR to permit intra-function BL.
To try to catch any remaining issues of this kind more reliably, this patch arranges that the result of EstimateFunctionSizeInBytes is saved in the ARMFunctionInfo, and in ARMConstantIslandPass, the estimate is checked against the true function size _unconditionally_. So underestimates should be detected even when they don't lead to an actual problem. Moreover, enough debug output is now generated to understand the basis of the estimate, and the true function size, and compare them to see why the estimate wasn't big enough.
No extra tests are added in this commit, because the existing tests of Thumb1 code generation in `llvm/test/CodeGen/Thumb` were already very good at catching failures of this check, and pointed out most of the detailed issues fixed in the previous patch.
>From cde8d0ad4324d7eec35e50eb9a46211ded2ce965 Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Mon, 8 Jun 2026 10:43:13 +0100
Subject: [PATCH] [ARM][Thumb1] Verify EstimateFunctionSizeInBytes every time
EstimateFunctionSizeInBytes is supposed to err on the side of
overestimating the function size, but historically, has sometimes
underestimated, leading to a code generation failure in
ARMConstantIslandPass in the rare case where the underestimate led
frame setup to fail to stack LR to permit intra-function BL.
To try to catch any remaining issues of this kind more reliably, this
patch arranges that the result of EstimateFunctionSizeInBytes is saved
in the ARMFunctionInfo, and in ARMConstantIslandPass, the estimate is
checked against the true function size _unconditionally_. So
underestimates should be detected even when they don't lead to an
actual problem. Moreover, enough debug output is now generated to
understand the basis of the estimate, and the true function size, and
compare them to see why the estimate wasn't big enough.
No extra tests are added in this commit, because the existing tests of
Thumb1 code generation in `llvm/test/CodeGen/Thumb` were already very
good at catching failures of this check, and pointed out most of the
detailed issues fixed in the previous patch.
---
llvm/lib/Target/ARM/ARMBasicBlockInfo.h | 4 +++
llvm/lib/Target/ARM/ARMConstantIslandPass.cpp | 30 +++++++++++++++++++
llvm/lib/Target/ARM/ARMFrameLowering.cpp | 21 +++++++++++--
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/ARM/ARMBasicBlockInfo.h b/llvm/lib/Target/ARM/ARMBasicBlockInfo.h
index daf8f9b4b8361..de4198b92e812 100644
--- a/llvm/lib/Target/ARM/ARMBasicBlockInfo.h
+++ b/llvm/lib/Target/ARM/ARMBasicBlockInfo.h
@@ -135,6 +135,10 @@ class ARMBasicBlockUtils {
return BBInfo[MBB->getNumber()].Offset;
}
+ unsigned getFunctionSize() const {
+ return BBInfo[MF.getNumBlockIDs() - 1].postOffset();
+ }
+
void adjustBBOffsetsAfter(MachineBasicBlock *MBB);
void adjustBBSize(MachineBasicBlock *MBB, int Size) {
diff --git a/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp b/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
index 97db51129cff2..27f89d9475eae 100644
--- a/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
+++ b/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
@@ -511,6 +511,36 @@ bool ARMConstantIslands::runOnMachineFunction(MachineFunction &mf) {
LLVM_DEBUG(dbgs() << '\n'; dumpBBs());
+ if (auto Estimate = AFI->getEstimatedFunctionSizeInBytes()) {
+ auto RealSize = BBUtils->getFunctionSize();
+ if (RealSize > *Estimate) {
+ LLVM_DEBUG({
+ dbgs() << "ARMConstantIslandsPass output for " << mf.getName()
+ << " with sizes:\n";
+ for (MachineBasicBlock &MBB : mf) {
+ unsigned Offset = BBUtils->getOffsetOf(&MBB);
+ unsigned End = BBUtils->getBBInfo()[MBB.getNumber()].postOffset();
+ dbgs() << printMBBReference(MBB) << ": // offset "
+ << Twine::utohexstr(Offset) << "\n";
+ for (MachineInstr &MI : MBB) {
+ unsigned InstSize = TII->getInstSizeInBytes(MI);
+ LLVM_DEBUG(dbgs() << " 0x" << Twine::utohexstr(Offset) << " +"
+ << InstSize << ": " << MI);
+ Offset += InstSize;
+ }
+ if (Offset < End) {
+ LLVM_DEBUG(dbgs() << " 0x" << Twine::utohexstr(Offset) << " +"
+ << (End - Offset) << ": extra\n");
+ }
+ }
+ });
+ report_fatal_error(
+ Twine("Underestimated size of function ") + mf.getName() +
+ ": estimate = " + Twine(*Estimate) +
+ ", size after ARMConstantIslandsPass = " + Twine(RealSize));
+ }
+ }
+
BBUtils->clear();
WaterList.clear();
CPUsers.clear();
diff --git a/llvm/lib/Target/ARM/ARMFrameLowering.cpp b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
index a282359e39dc6..b08bf45e6e8e5 100644
--- a/llvm/lib/Target/ARM/ARMFrameLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
@@ -2351,10 +2351,14 @@ static unsigned EstimateFunctionSizeInBytes(const MachineFunction &MF,
bool BigFrameOffsets) {
unsigned FnSize = 0;
+ LLVM_DEBUG(dbgs() << "EstimateFunctionSizeInBytes(" << MF.getName()
+ << "):\n");
+
if (MF.shouldSplitStack()) {
// Split stack prologue saves r4,r5; makes a copy of sp and loads
// a literal; compares the two, and if sp < literal, pushes
// further registers and calls __morestack.
+ LLVM_DEBUG(dbgs() << " +0x24 bytes for split stack\n");
FnSize += 0x24;
}
@@ -2365,6 +2369,7 @@ static unsigned EstimateFunctionSizeInBytes(const MachineFunction &MF,
// + stack update (up to 6 bytes)
// + stack realignment (8)
// + make base pointer (2).
+ LLVM_DEBUG(dbgs() << " +0x38 bytes for prologue\n");
FnSize += 0x38;
// Size of a large epilogue:
@@ -2374,9 +2379,11 @@ static unsigned EstimateFunctionSizeInBytes(const MachineFunction &MF,
// + pop return address into a low reg (2 bytes)
// + update sp to undo variadic function setup (2 bytes)
// + BX to where you popped the return address (2 bytes)
+ LLVM_DEBUG(dbgs() << " +0x1e bytes for epilogue\n");
FnSize += 0x1e;
for (auto &MBB : MF) {
+ LLVM_DEBUG(dbgs() << " " << printMBBReference(MBB) << ":\n");
bool seenBranch = false, seenConstantLoad = false;
for (auto &MI : MBB) {
unsigned InstSize;
@@ -2450,6 +2457,8 @@ static unsigned EstimateFunctionSizeInBytes(const MachineFunction &MF,
InstSize = TII.getInstSizeInBytes(MI);
break;
}
+ LLVM_DEBUG(dbgs() << " 0x" << Twine::utohexstr(FnSize) << " +"
+ << InstSize << ": " << MI);
FnSize += InstSize;
@@ -2461,6 +2470,8 @@ static unsigned EstimateFunctionSizeInBytes(const MachineFunction &MF,
MO->getPointerInfo().V);
if (PSV && PSV->kind() == PseudoSourceValue::ConstantPool) {
unsigned ConstSize = MO->getType().getSizeInBytes();
+ LLVM_DEBUG(dbgs() << " 0x" << Twine::utohexstr(FnSize) << " +"
+ << ConstSize << ": constant pool entry\n");
FnSize += ConstSize;
seenConstantLoad = true;
}
@@ -2479,13 +2490,18 @@ static unsigned EstimateFunctionSizeInBytes(const MachineFunction &MF,
// We might have to realign at the end of a basic block.
FnSize += 2;
}
+ LLVM_DEBUG(dbgs() << " trailers:\n");
if (MF.getJumpTableInfo()) {
for (auto &Table : MF.getJumpTableInfo()->getJumpTables()) {
unsigned TableLen = Table.MBBs.size();
unsigned TableSizeBytes = TableLen * 4;
+ LLVM_DEBUG(dbgs() << " 0x" << Twine::utohexstr(FnSize) << " +"
+ << TableSizeBytes << ": jump table\n");
FnSize += TableSizeBytes;
}
}
+ LLVM_DEBUG(dbgs() << "Estimated function size for " << MF.getName() << " = "
+ << FnSize << " bytes\n");
return FnSize;
}
@@ -2930,11 +2946,12 @@ void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,
<< "; EstimatedFPStack: " << MaxFixedOffset - MaxFPOffset
<< "; BigFrameOffsets: " << BigFrameOffsets << "\n");
- if (!LRSpilled && AFI->isThumb1OnlyFunction()) {
+ if (AFI->isThumb1OnlyFunction()) {
unsigned FnSize =
EstimateFunctionSizeInBytes(MF, TII, STI, BigFrameOffsets);
+ AFI->setEstimatedFunctionSizeInBytes(FnSize);
- if (FnSize >= (1 << 11)) {
+ if (!LRSpilled && FnSize >= (1 << 11)) {
// Force LR to be spilled if the Thumb function size is > 2048. This
// enables use of BL to implement far jump.
CanEliminateFrame = false;
More information about the llvm-branch-commits
mailing list