This is an archive of the discontinued LLVM Phabricator instance.

[BOLT] Instrumentation: AArch64 instrumentation support in runtime
ClosedPublic

Authored by Elvina on Jun 1 2023, 2:37 PM.

Details

Summary

This patch adds support for AArch64 in instrumentation runtime library, including AArch64 system calls.
Also this commit divides syscalls into target-specific files.

Elvina Yakubova,
Advanced Software Technology Lab, Huawei

Diff Detail

Event Timeline

Elvina created this revision.Jun 1 2023, 2:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2023, 2:37 PM
Elvina requested review of this revision.Jun 1 2023, 2:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2023, 2:37 PM
Elvina updated this revision to Diff 527947.Jun 2 2023, 1:32 PM

clang-format

rafauler added inline comments.Jun 26 2023, 5:49 PM
bolt/CMakeLists.txt
41–42

We don't support runtime on windows, so we shouldn't delete these checks. Just change the x86 check to check for both x86 OR AArch64

bolt/runtime/instr.cpp
1671

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.

1712

same

1717

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.

Elvina marked 2 inline comments as done.Jun 30 2023, 7:47 AM
Elvina added inline comments.
bolt/runtime/instr.cpp
1671

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

1717

Thanks for suggesting it, that's exactly what I needed :)

Elvina updated this revision to Diff 536249.Jun 30 2023, 7:51 AM
Elvina marked an inline comment as done.

Fixed Cmake, added clang-format plugs

Elvina updated this revision to Diff 536265.Jun 30 2023, 8:29 AM
Elvina updated this revision to Diff 536267.Jun 30 2023, 8:32 AM
treapster added inline comments.Jun 30 2023, 9:11 AM
bolt/runtime/CMakeLists.txt
32

Drop if, use -mgeneral-regs-only

Elvina updated this revision to Diff 536297.Jun 30 2023, 9:48 AM
Elvina marked an inline comment as done.
Elvina added inline comments.
bolt/runtime/CMakeLists.txt
32

thanks for noticing, forgot about it

I wonder why is this diff not showing up on top of the D151899 diff stack anymore? Isn't it dependent on that stack?

Elvina marked an inline comment as done.Jul 6 2023, 11:13 PM

I wonder why is this diff not showing up on top of the D151899 diff stack anymore? Isn't it dependent on that stack?

Yes, it is, added it.

Gentle ping

Elvina updated this revision to Diff 548228.Aug 8 2023, 7:59 AM

add check for Darwin OS, so we'll not get error during building with clang on Linux (see here https://github.com/llvm/llvm-project/issues/63255#issuecomment-1589716433)

rafauler added inline comments.Aug 9 2023, 5:39 PM
bolt/runtime/CMakeLists.txt
30

Dropping -mno-sse is causing problems for me on my x86 machine.

test runtime/X86/instrumentation-xmm.c is failing

Elvina marked an inline comment as done.Aug 10 2023, 8:05 AM
Elvina added inline comments.
bolt/runtime/CMakeLists.txt
30

Thanks for noticing!
Turned out general-regs-only doesn't disable sse on clang https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Arch/X86.cpp#L245

Elvina updated this revision to Diff 549051.Aug 10 2023, 8:08 AM
Elvina marked an inline comment as done.

Setting -mno-sse flag back

rafauler accepted this revision.Aug 18 2023, 5:04 PM

Thanks Elvina! LGTM

This revision is now accepted and ready to land.Aug 18 2023, 5:04 PM
Elvina updated this revision to Diff 553052.Aug 24 2023, 2:51 AM

rebase + add syscalls for AArch64

yota9 accepted this revision.Aug 24 2023, 8:36 AM

LGTM

@rafauler Thanks for reviewing this!

phosek added a subscriber: phosek.Aug 25 2023, 11:14 AM

This change broke our builder that's cross-compiling the toolchain from x86_64-linux-gnu to aarch64-linux-gnu. It looks like the bolt_rt build is now trying to execute the cross-compiled clang and fails because it's no possible to run the aarch64-linux-gnu binary on x86_64-linux-gnu.

bolt/CMakeLists.txt
35–38

This probably also needs to check that CMAKE_CROSSCOMPILING isn't set (that is NOT CMAKE_CROSSCOMPILING).

rafauler added inline comments.Aug 25 2023, 4:09 PM
bolt/CMakeLists.txt
35–38

Would this work? D158906

@rafauler Thanks for fixing this. I noticed the issue just now