[PATCH] D156389: [BOLT] Add support for instrumenting conditional tail calls

[Amir Ayupov via Phabricator via llvm-commits]

Amir updated this revision to Diff 545242.
Amir added a comment.

Move Offset annotation from former CTC to the tail call.

We identify instructions to be instrumented based on Offset annotation.

For expanded CTC we keep Offset attached to the original instruction which is
converted into a regular conditional jump, while leaving the newly created tail
call without an Offset annotation. This leads to attempting the instrumentation
of the conditional jump which points to the basic block with an inherited input
offset thus creating an invalid edge description. At the same time, the newly
created tail call is skipped entirely which means we're not creating a call
description for it.

If we instead reassign Offset annotation from the conditional jump to the tail
call we fix both issues. The conditional jump will be skipped not creating an
invalid edge description, while tail call will be handled properly (unformly
with regular calls).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156389/new/

https://reviews.llvm.org/D156389

Files:
  bolt/lib/Core/BinaryFunction.cpp
  bolt/test/runtime/X86/instrumentation-tail-call.s


Index: bolt/test/runtime/X86/instrumentation-tail-call.s
===================================================================
--- bolt/test/runtime/X86/instrumentation-tail-call.s
+++ bolt/test/runtime/X86/instrumentation-tail-call.s
@@ -14,6 +14,9 @@
 
 # CHECK: leaq 0x80(%rsp), %rsp
 
+# RUN: FileCheck %s --input-file %t.fdata --check-prefix=CHECK-FDATA
+# CHECK-FDATA: 1 main {{.*}} 1 targetFunc 0 0 1
+
   .text
   .globl  main
   .type main, %function
@@ -32,7 +35,8 @@
   movq %rbp, %rsp
   pop %rbp
   mov -0x10(%rsp),%rax
-  jmp targetFunc
+  test %rsp, %rsp
+  jne targetFunc
 
 .LBBerror:
   addq $0x20, %rsp
Index: bolt/lib/Core/BinaryFunction.cpp
===================================================================
--- bolt/lib/Core/BinaryFunction.cpp
+++ bolt/lib/Core/BinaryFunction.cpp
@@ -2305,6 +2305,12 @@
 
     // This branch is no longer a conditional tail call.
     BC.MIB->unsetConditionalTailCall(*CTCInstr);
+
+    // Move offset from CTCInstr to TailCallInstr.
+    if (std::optional<uint32_t> Offset = BC.MIB->getOffset(*CTCInstr)) {
+      BC.MIB->setOffset(TailCallInstr, *Offset);
+      BC.MIB->clearOffset(*CTCInstr);
+    }
   }
 
   insertBasicBlocks(std::prev(end()), std::move(NewBlocks),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D156389.545242.patch
Type: text/x-patch
Size: 1227 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230728/81bcb52a/attachment.bin>