[PATCH] D151899: [BOLT] Instrumentation: Initial instrumentation support for AArch64

[Elvina Yakubova via Phabricator via llvm-commits]

Elvina marked 10 inline comments as done.
Elvina added a comment.

Thanks for reviewing this! Fixed everything you mentioned



================
Comment at: bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp:1345
+  void convertIndirectCallToLoad(MCInst &Inst, MCPhysReg Reg) override {
+    if (Inst.getOpcode() == AArch64::BL || Inst.getOpcode() == AArch64::BLR ||
+        Inst.getOpcode() == AArch64::BR || Inst.getOpcode() == AArch64::B) {
----------------
rafauler wrote:
> Don't we need to remove the tailcall annotation like in the X86 code?
right, thanks for pointing this


================
Comment at: bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp:1490
+    I += Addr.size();
+    storeReg(Instrs[I++], AArch64::X2, getSpRegister(CodePointerSize));
+    InstructionListType Insts = createIncMemory(AArch64::X0, AArch64::X2);
----------------
rafauler wrote:
> Do we support 32-bit pointers for AArch64? If not, I would just hardcode AArch64::SP here and in other locations an drop the CodePointerSize argument. If we do support, I think it would be nice to have tests that show we are working correctly for these binaries.
currently no, so SP is enough


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

https://reviews.llvm.org/D151899