Skip to content

Conversation

@aaupov
Copy link
Contributor

@aaupov aaupov commented May 12, 2024

Offset annotation was missed when optimizing an unconditional branch to
a tail call.

Test Plan: update bb-with-two-tail-calls.s

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented May 12, 2024

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

Offset annotation was missed when optimizing an unconditional branch to
a tail call.

Test Plan: update bb-with-two-tail-calls.s


Full diff: https://github.com/llvm/llvm-project/pull/91898.diff

2 Files Affected:

  • (modified) bolt/lib/Passes/BinaryPasses.cpp (+3)
  • (modified) bolt/test/X86/bb-with-two-tail-calls.s (+3-3)
diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index df6dbcddeed56..867f977cebca7 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -715,6 +715,9 @@ 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());
         } else {
           return false;
         }
diff --git a/bolt/test/X86/bb-with-two-tail-calls.s b/bolt/test/X86/bb-with-two-tail-calls.s
index caad7b3d735f5..bb2b0cd4cc23a 100644
--- a/bolt/test/X86/bb-with-two-tail-calls.s
+++ b/bolt/test/X86/bb-with-two-tail-calls.s
@@ -9,11 +9,11 @@
 # RUN: llvm-strip --strip-unneeded %t.o
 # RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -nostdlib
 # RUN: llvm-bolt %t.exe -o %t.out --data %t.fdata --lite=0 --dyno-stats \
-# RUN:    --print-sctc --print-only=_start 2>&1 | FileCheck %s
+# RUN:    --print-sctc --print-only=_start -enable-bat 2>&1 | FileCheck %s
 # 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  # CTCTakenCount: {{.*}}
-# CHECK-NEXT:    {{.*}}:   jmp     {{.*}} # TAILCALL
+# CHECK:         {{.*}}:   ja      {{.*}} # TAILCALL # Offset: 7 # CTCTakenCount: 4
+# CHECK-NEXT:    {{.*}}:   jmp     {{.*}} # TAILCALL # Offset: 12
 
   .globl _start
 _start:

Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

Created using spr 1.3.4
@aaupov aaupov merged commit 47a68c8 into users/aaupov/spr/main.bolt-preserve-offset-annotation-in-fixdoublejumps May 12, 2024
@aaupov aaupov deleted the users/aaupov/spr/bolt-preserve-offset-annotation-in-fixdoublejumps branch May 12, 2024 21:26
aaupov added a commit that referenced this pull request May 13, 2024
Offset annotation was missed when optimizing an unconditional branch to
a tail call.

Test Plan: update bb-with-two-tail-calls.s
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants