[llvm] [AsmPrinter] Add generic support for verifying instruction sizes (PR #187703)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 20 07:09:40 PDT 2026
https://github.com/nikic created https://github.com/llvm/llvm-project/pull/187703
Many backends rely on TII reporting correct instruction sizes for MIR level branch relaxation passes. Reporting a too small size can result in MC fixup failures (or silent miscompiles for unvalidated fixups).
Some time ago I added validation to the PPC asm printer to verify that the TII instruction size matches the actually emitted size. This was very helpful to systematically fix all incorrectly reported instruction sizes.
However, the same problem also exists in lots of other backens, so this moves the validation into AsmPrinter, controlled by a new shouldVerifyInstSize() hook in TII, which is disabled by default.
The intention here is to gradually enable this validation for more backends (which requires fixing them first).
>From 8c2e7ec30f77780af5ccfa0c90953fb39f083a7b Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Fri, 20 Mar 2026 14:38:29 +0100
Subject: [PATCH] [AsmPrinter] Add generic support for verifying instruction
sizes
Many backends rely on TII reporting correct instruction sizes for
MIR level branch relaxation passes. Reporting a too small size can
result in MC fixup failures (or silent miscompiles for unvalidated
fixups).
Some time ago I added validation to the PPC asm printer to verify
that the TII instruction size matches the actually emitted size.
This was very helpful to systematically fix all incorrectly reported
instruction sizes.
However, the same problem also exists in lots of other backens,
so this moves the validation into AsmPrinter, controlled by a new
shouldVerifyInstSize() hook in TII, which is disabled by default.
The intention here is to gradually enable this validation for more
backends (which requires fixing them first).
---
llvm/include/llvm/CodeGen/TargetInstrInfo.h | 6 +++++
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 28 +++++++++++++++++++++
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp | 26 -------------------
llvm/lib/Target/PowerPC/PPCInstrInfo.cpp | 5 ++++
llvm/lib/Target/PowerPC/PPCInstrInfo.h | 2 ++
5 files changed, 41 insertions(+), 26 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 3910e77d13de7..9affa66040caa 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -428,6 +428,12 @@ class LLVM_ABI TargetInstrInfo : public MCInstrInfo {
return ~0U;
}
+ /// Whether the correctness of the instruction size returned by
+ /// getInstSizeInBytes() should be verified.
+ virtual bool shouldVerifyInstSize(const MachineInstr &MI) const {
+ return false;
+ }
+
/// Return true if the instruction is as cheap as a move instruction.
///
/// Targets for different archs need to override this, and different
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index db4fc5888f6d4..322dc9d8fb92c 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -2147,6 +2147,11 @@ void AsmPrinter::emitFunctionBody() {
if (isVerbose())
emitComments(MI, STI, OutStreamer->getCommentOS());
+#ifndef NDEBUG
+ MCFragment *OldFragment = OutStreamer->getCurrentFragment();
+ size_t OldFragSize = OldFragment->getFixedSize();
+#endif
+
switch (MI.getOpcode()) {
case TargetOpcode::CFI_INSTRUCTION:
emitCFIInstruction(MI);
@@ -2265,6 +2270,29 @@ void AsmPrinter::emitFunctionBody() {
break;
}
+#ifndef NDEBUG
+ // Verify that the instruction size reported by InstrInfo matches the
+ // actually emitted size. Many backends performing branch relaxation
+ // on the MIR level rely on this for correctness.
+ if (OutStreamer->isObj()) {
+ const TargetInstrInfo *TII = MF->getSubtarget().getInstrInfo();
+ MCFragment *NewFragment = OutStreamer->getCurrentFragment();
+ // Don't try to handle fragment splitting cases.
+ if (NewFragment == OldFragment && TII->shouldVerifyInstSize(MI)) {
+ unsigned ExpectedSize = TII->getInstSizeInBytes(MI);
+ unsigned ActualSize = NewFragment->getFixedSize() - OldFragSize;
+ // FIXME: InstrInfo currently over-estimates the size of STACKMAP.
+ if (ActualSize != ExpectedSize &&
+ MI.getOpcode() != TargetOpcode::STACKMAP) {
+ dbgs() << "Size mismatch for: " << MI << "\n";
+ dbgs() << "Expected size: " << ExpectedSize << "\n";
+ dbgs() << "Actual size: " << ActualSize << "\n";
+ abort();
+ }
+ }
+ }
+#endif
+
if (MI.isCall()) {
if (MF->getTarget().Options.BBAddrMap)
OutStreamer->emitLabel(createCallsiteEndSymbol(MBB));
diff --git a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
index f5d56c1e8e264..6d1ee6bb111fe 100644
--- a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -27,7 +27,6 @@
#include "PPCTargetMachine.h"
#include "TargetInfo/PowerPCTargetInfo.h"
#include "llvm/ADT/MapVector.h"
-#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/StringExtras.h"
@@ -946,31 +945,6 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr *MI) {
return PPC::S_None;
};
-#ifndef NDEBUG
- // Instruction sizes must be correct for PPCBranchSelector to pick the
- // right branch kind. Verify that the reported sizes and the actually
- // emitted sizes match.
- unsigned ExpectedSize = Subtarget->getInstrInfo()->getInstSizeInBytes(*MI);
- MCFragment *OldFragment = OutStreamer->getCurrentFragment();
- size_t OldFragSize = OldFragment->getFixedSize();
- scope_exit VerifyInstSize([&]() {
- if (!OutStreamer->isObj())
- return; // Can only verify size when streaming to object.
- MCFragment *NewFragment = OutStreamer->getCurrentFragment();
- if (NewFragment != OldFragment)
- return; // Don't try to handle fragment splitting cases.
- unsigned ActualSize = NewFragment->getFixedSize() - OldFragSize;
- // FIXME: InstrInfo currently over-estimates the size of STACKMAP.
- if (ActualSize != ExpectedSize &&
- MI->getOpcode() != TargetOpcode::STACKMAP) {
- dbgs() << "Size mismatch for: " << *MI << "\n";
- dbgs() << "Expected size: " << ExpectedSize << "\n";
- dbgs() << "Actual size: " << ActualSize << "\n";
- abort();
- }
- });
-#endif
-
// Lower multi-instruction pseudo operations.
switch (MI->getOpcode()) {
default: break;
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
index 278fd37d4c229..e9a89930bb8d7 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -3034,6 +3034,11 @@ unsigned PPCInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
}
}
+bool PPCInstrInfo::shouldVerifyInstSize(const MachineInstr &MI) const {
+ // FIXME: The size of STACKMAP is currently over-estimated.
+ return MI.getOpcode() != TargetOpcode::STACKMAP;
+}
+
std::pair<unsigned, unsigned>
PPCInstrInfo::decomposeMachineOperandsTargetFlags(unsigned TF) const {
// PPC always uses a direct mask.
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.h b/llvm/lib/Target/PowerPC/PPCInstrInfo.h
index 9fe5706e7b1e0..320cc94bebfd7 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.h
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.h
@@ -700,6 +700,8 @@ class PPCInstrInfo : public PPCGenInstrInfo {
///
unsigned getInstSizeInBytes(const MachineInstr &MI) const override;
+ bool shouldVerifyInstSize(const MachineInstr &MI) const override;
+
MCInst getNop() const override;
std::pair<unsigned, unsigned>
More information about the llvm-commits
mailing list