[llvm-branch-commits] [llvm] [ARM][Thumb1] Verify EstimateFunctionSizeInBytes every time (PR #203319)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Jun 11 09:06:50 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-arm
Author: Simon Tatham (statham-arm)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/203319.diff
3 Files Affected:
- (modified) llvm/lib/Target/ARM/ARMBasicBlockInfo.h (+4)
- (modified) llvm/lib/Target/ARM/ARMConstantIslandPass.cpp (+30)
- (modified) llvm/lib/Target/ARM/ARMFrameLowering.cpp (+19-2)
``````````diff
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;
``````````
</details>
https://github.com/llvm/llvm-project/pull/203319
More information about the llvm-branch-commits
mailing list