[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
Fri Jun 12 02:17:58 PDT 2026
https://github.com/statham-arm updated https://github.com/llvm/llvm-project/pull/203319
>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 1/2] [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;
>From 6b078e995152939d35dea3ad13ecb4a4c00b2aa0 Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Fri, 12 Jun 2026 10:17:26 +0100
Subject: [PATCH 2/2] Update an existing test for new more precise diagnostics
---
llvm/test/CodeGen/ARM/estimate-size-copy.mir | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/test/CodeGen/ARM/estimate-size-copy.mir b/llvm/test/CodeGen/ARM/estimate-size-copy.mir
index 154117d1743d1..3665493b644d5 100644
--- a/llvm/test/CodeGen/ARM/estimate-size-copy.mir
+++ b/llvm/test/CodeGen/ARM/estimate-size-copy.mir
@@ -4,7 +4,7 @@
# RUN: FileCheck %s --check-prefix=OUTPUT
# RUN: FileCheck %s --check-prefix=DEBUG < %t
#
-# DEBUG: Estimated function size for f = 4 bytes
+# DEBUG: 0x{{[0-9a-f]+}} +4: renamable $r0 = COPY $r1
#
# OUTPUT: mov r0, r1
# OUTPUT: bx lr
More information about the llvm-branch-commits
mailing list