[llvm] [BOLT] Offset LPStart to avoid unnecessary instructions (PR #116713)
Maksim Panchenko via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 19 16:44:40 PST 2024
https://github.com/maksfb updated https://github.com/llvm/llvm-project/pull/116713
>From fbd363e947b913c63f1595a7f109a7a836521cfd Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Mon, 18 Nov 2024 15:06:09 -0800
Subject: [PATCH 1/2] [BOLT] Offset LPStart to avoid unnecessary instructions
For C++ exception handling, when we write a call site table, we must
avoid emitting 0-value offsets for landing pads unless the call site has
no landing pad. However, 0 can be a real offset from the start of the
FDE if the FDE corresponds to a function fragment that starts with a
landing pad. In such cases, we used to emit a trap instruction at the
start of the fragment to guarantee non-zero LP offset.
To avoid emitting unnecessary trap instructions, we can instead set
LPStart to an offset from the FDE. If we emit it as [FDEStart - 1], then
all real offsets from LPStart in FDE become non-negative.
---
bolt/lib/Core/BinaryEmitter.cpp | 70 +++++++++++++++++----------------
1 file changed, 37 insertions(+), 33 deletions(-)
diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index bdf784ec7b6f3e..08590d3995342d 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -416,17 +416,6 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF,
BF.duplicateConstantIslands();
}
- if (!FF.empty() && FF.front()->isLandingPad()) {
- assert(!FF.front()->isEntryPoint() &&
- "Landing pad cannot be entry point of function");
- // If the first block of the fragment is a landing pad, it's offset from the
- // start of the area that the corresponding LSDA describes is zero. In this
- // case, the call site entries in that LSDA have 0 as offset to the landing
- // pad, which the runtime interprets as "no handler". To prevent this,
- // insert some padding.
- Streamer.emitBytes(BC.MIB->getTrapFillValue());
- }
-
// Track the first emitted instruction with debug info.
bool FirstInstr = true;
for (BinaryBasicBlock *const BB : FF) {
@@ -926,39 +915,54 @@ void BinaryEmitter::emitLSDA(BinaryFunction &BF, const FunctionFragment &FF) {
// Emit the LSDA header.
// If LPStart is omitted, then the start of the FDE is used as a base for
- // landing pad displacements. Then if a cold fragment starts with
- // a landing pad, this means that the first landing pad offset will be 0.
- // As a result, the exception handling runtime will ignore this landing pad
- // because zero offset denotes the absence of a landing pad.
- // For this reason, when the binary has fixed starting address we emit LPStart
- // as 0 and output the absolute value of the landing pad in the table.
+ // landing pad displacements. Then, if a cold fragment starts with a landing
+ // pad, this means that the first landing pad offset will be 0. However, C++
+ // runtime treats 0 as if there is no landing pad present, thus we *must* emit
+ // non-zero offsets for all valid LPs.
//
- // If the base address can change, we cannot use absolute addresses for
- // landing pads (at least not without runtime relocations). Hence, we fall
- // back to emitting landing pads relative to the FDE start.
- // As we are emitting label differences, we have to guarantee both labels are
- // defined in the same section and hence cannot place the landing pad into a
- // cold fragment when the corresponding call site is in the hot fragment.
- // Because of this issue and the previously described issue of possible
- // zero-offset landing pad we have to place landing pads in the same section
- // as the corresponding invokes for shared objects.
+ // As a solution, for fixed-address binaries we set LPStart to 0, and for
+ // position-independent binaries we set LP start to FDE start minus one byte
+ // for FDEs that start with a landing pad.
std::function<void(const MCSymbol *)> emitLandingPad;
if (BC.HasFixedLoadAddress) {
Streamer.emitIntValue(dwarf::DW_EH_PE_udata4, 1); // LPStart format
Streamer.emitIntValue(0, 4); // LPStart
emitLandingPad = [&](const MCSymbol *LPSymbol) {
- if (!LPSymbol)
- Streamer.emitIntValue(0, 4);
- else
+ if (LPSymbol)
Streamer.emitSymbolValue(LPSymbol, 4);
+ else
+ Streamer.emitIntValue(0, 4);
};
} else {
- Streamer.emitIntValue(dwarf::DW_EH_PE_omit, 1); // LPStart format
+ const bool NeedsLPAdjustment = !FF.empty() && FF.front()->isLandingPad();
+ if (NeedsLPAdjustment) {
+ // Use relative LPStart format and emit LPStart as [SymbolStart - 1].
+ Streamer.emitIntValue(dwarf::DW_EH_PE_pcrel | dwarf::DW_EH_PE_sdata4, 1);
+ MCSymbol *DotSymbol = BC.Ctx->createTempSymbol("LPBase");
+ Streamer.emitLabel(DotSymbol);
+
+ const MCExpr *LPStartExpr = MCBinaryExpr::createSub(
+ MCSymbolRefExpr::create(StartSymbol, *BC.Ctx),
+ MCSymbolRefExpr::create(DotSymbol, *BC.Ctx), *BC.Ctx);
+ LPStartExpr = MCBinaryExpr::createSub(
+ LPStartExpr, MCConstantExpr::create(1, *BC.Ctx), *BC.Ctx);
+ Streamer.emitValue(LPStartExpr, 4);
+ } else {
+ // DW_EH_PE_omit means FDE start (StartSymbol) will be used as LPStart.
+ Streamer.emitIntValue(dwarf::DW_EH_PE_omit, 1);
+ }
emitLandingPad = [&](const MCSymbol *LPSymbol) {
- if (!LPSymbol)
+ if (LPSymbol) {
+ const MCExpr *LPOffsetExpr = MCBinaryExpr::createSub(
+ MCSymbolRefExpr::create(LPSymbol, *BC.Ctx),
+ MCSymbolRefExpr::create(StartSymbol, *BC.Ctx), *BC.Ctx);
+ if (NeedsLPAdjustment)
+ LPOffsetExpr = MCBinaryExpr::createAdd(
+ LPOffsetExpr, MCConstantExpr::create(1, *BC.Ctx), *BC.Ctx);
+ Streamer.emitValue(LPOffsetExpr, 4);
+ } else {
Streamer.emitIntValue(0, 4);
- else
- Streamer.emitAbsoluteSymbolDiff(LPSymbol, StartSymbol, 4);
+ }
};
}
>From a9cc2af396f6ab7daf41e8d41f34503037ab71de Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Tue, 19 Nov 2024 16:44:29 -0800
Subject: [PATCH 2/2] fixup! [BOLT] Offset LPStart to avoid unnecessary
instructions
---
bolt/lib/Core/BinaryEmitter.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index 08590d3995342d..408663180935c2 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -923,6 +923,7 @@ void BinaryEmitter::emitLSDA(BinaryFunction &BF, const FunctionFragment &FF) {
// As a solution, for fixed-address binaries we set LPStart to 0, and for
// position-independent binaries we set LP start to FDE start minus one byte
// for FDEs that start with a landing pad.
+ const bool NeedsLPAdjustment = !FF.empty() && FF.front()->isLandingPad();
std::function<void(const MCSymbol *)> emitLandingPad;
if (BC.HasFixedLoadAddress) {
Streamer.emitIntValue(dwarf::DW_EH_PE_udata4, 1); // LPStart format
@@ -934,7 +935,6 @@ void BinaryEmitter::emitLSDA(BinaryFunction &BF, const FunctionFragment &FF) {
Streamer.emitIntValue(0, 4);
};
} else {
- const bool NeedsLPAdjustment = !FF.empty() && FF.front()->isLandingPad();
if (NeedsLPAdjustment) {
// Use relative LPStart format and emit LPStart as [SymbolStart - 1].
Streamer.emitIntValue(dwarf::DW_EH_PE_pcrel | dwarf::DW_EH_PE_sdata4, 1);
More information about the llvm-commits
mailing list