[PATCH] D151942: [BOLT] Instrumentation: AArch64 instrumentation support in runtime

Fri Jun 30 07:48:00 PDT 2023

Elvina marked 2 inline comments as done.
Elvina added inline comments.

================
Comment at: bolt/runtime/instr.cpp:1591
+#if defined(__aarch64__)
+  __asm__ __volatile__(SAVE_ALL "ldp x0, x1, [sp, #288]\n"
+                                "bl instrumentIndirectCall\n" RESTORE_ALL
----------------
rafauler wrote:
> I have the impression that the correct offset here is 256 instead of 288, but I can't test this. That's why I asked for a test for indirect calls in the other diff.
We have several stp/str instructions before that, when we're adding instrumentation snippets in createIndirectCallIst and createInstrumentedIndCallHandlerEntryBB functions, that's where 32 is added from

================
Comment at: bolt/runtime/instr.cpp:1614
+#else
+  __asm__ __volatile__(SAVE_ALL "mov 0x98(%%rsp), %%rdi\n"
+                                "mov 0x90(%%rsp), %%rsi\n"
----------------
rafauler wrote:
> clang format is doing the wrong thing here putting SAVE_ALL in the same line as other assembly instructions.
> 
> I think we should use: 
>   // clang-format off
> 
> and
>   // clang-format on
> 
> surrounding this code, and put SAVE_ALL in a separate line like before this diff.
Thanks for suggesting it, that's exactly what I needed :)

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

https://reviews.llvm.org/D151942