[llvm] [BOLT] Fix preserved offset in fixDoubleJumps (PR #92485)
Amir Ayupov via llvm-commits
llvm-commits at lists.llvm.org
Thu May 16 19:06:42 PDT 2024
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/92485
The offset was inherited from the containing basic block rather than
from branch instruction.
Test Plan: updated bolt/test/X86/bb-with-two-tail-calls.s
>From 6bfa8fbb1d33c458a8cb55242d1032d308aae965 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Wed, 15 May 2024 09:25:19 -0700
Subject: [PATCH] [BOLT] Fix preserved offset in fixDoubleJumps
The offset was inherited from the containing basic block rather than
from branch instruction.
Test Plan: updated bolt/test/X86/bb-with-two-tail-calls.s
---
bolt/lib/Passes/BinaryPasses.cpp | 14 +++++++++-----
bolt/test/X86/bb-with-two-tail-calls.s | 8 ++++----
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index 867f977cebca7..298ba29ff5b3f 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -674,7 +674,8 @@ static uint64_t fixDoubleJumps(BinaryFunction &Function, bool MarkInvalid) {
MCPlusBuilder *MIB = Function.getBinaryContext().MIB.get();
for (BinaryBasicBlock &BB : Function) {
auto checkAndPatch = [&](BinaryBasicBlock *Pred, BinaryBasicBlock *Succ,
- const MCSymbol *SuccSym) {
+ const MCSymbol *SuccSym,
+ std::optional<uint32_t> Offset) {
// Ignore infinite loop jumps or fallthrough tail jumps.
if (Pred == Succ || Succ == &BB)
return false;
@@ -715,9 +716,11 @@ static uint64_t fixDoubleJumps(BinaryFunction &Function, bool MarkInvalid) {
Pred->removeSuccessor(&BB);
Pred->eraseInstruction(Pred->findInstruction(Branch));
Pred->addTailCallInstruction(SuccSym);
- MCInst *TailCall = Pred->getLastNonPseudoInstr();
- assert(TailCall);
- MIB->setOffset(*TailCall, BB.getOffset());
+ if (Offset) {
+ MCInst *TailCall = Pred->getLastNonPseudoInstr();
+ assert(TailCall);
+ MIB->setOffset(*TailCall, *Offset);
+ }
} else {
return false;
}
@@ -760,7 +763,8 @@ static uint64_t fixDoubleJumps(BinaryFunction &Function, bool MarkInvalid) {
if (Pred->getSuccessor() == &BB ||
(Pred->getConditionalSuccessor(true) == &BB && !IsTailCall) ||
Pred->getConditionalSuccessor(false) == &BB)
- if (checkAndPatch(Pred, Succ, SuccSym) && MarkInvalid)
+ if (checkAndPatch(Pred, Succ, SuccSym, MIB->getOffset(*Inst)) &&
+ MarkInvalid)
BB.markValid(BB.pred_size() != 0 || BB.isLandingPad() ||
BB.isEntryPoint());
}
diff --git a/bolt/test/X86/bb-with-two-tail-calls.s b/bolt/test/X86/bb-with-two-tail-calls.s
index bb2b0cd4cc23a..b6703e352ff4b 100644
--- a/bolt/test/X86/bb-with-two-tail-calls.s
+++ b/bolt/test/X86/bb-with-two-tail-calls.s
@@ -1,8 +1,6 @@
# This reproduces a bug with dynostats when trying to compute branch stats
# at a block with two tails calls (one conditional and one unconditional).
-# REQUIRES: system-linux
-
# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown \
# RUN: %s -o %t.o
# RUN: link_fdata %s %t.o %t.fdata
@@ -13,7 +11,7 @@
# CHECK-NOT: Assertion `BranchInfo.size() == 2 && "could only be called for blocks with 2 successors"' failed.
# Two tail calls in the same basic block after SCTC:
# CHECK: {{.*}}: ja {{.*}} # TAILCALL # Offset: 7 # CTCTakenCount: 4
-# CHECK-NEXT: {{.*}}: jmp {{.*}} # TAILCALL # Offset: 12
+# CHECK-NEXT: {{.*}}: jmp {{.*}} # TAILCALL # Offset: 13
.globl _start
_start:
@@ -23,7 +21,9 @@ a: ja b
x: ret
# FDATA: 1 _start #a# 1 _start #b# 2 4
b: jmp e
-c: jmp f
+c:
+ .nops 1
+ jmp f
.globl e
e:
More information about the llvm-commits
mailing list