[llvm] [BOLT][RISCV] Carry-over annotations when fixing calls (PR #66763)
Job Noorman via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 20 23:36:55 PDT 2023
https://github.com/mtvec updated https://github.com/llvm/llvm-project/pull/66763
>From 52fb718c2942e39d35950520846273e144487df8 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Tue, 19 Sep 2023 13:25:29 +0200
Subject: [PATCH 1/2] [BOLT][RISCV] Carry-over annotations when fixing calls
`FixRISCVCallsPass` changes all different forms of calls to `PseudoCALL`
instructions. However, the original call's annotations were lost in the
process.
This patch fixes this by moving all annotations from the old to the new
call. `MCPlusBuilder::moveAnnotations` had to be made public for this.
---
bolt/include/bolt/Core/MCPlusBuilder.h | 2 +-
bolt/lib/Passes/FixRISCVCallsPass.cpp | 13 ++++++++++
bolt/test/RISCV/call-annotations.s | 33 ++++++++++++++++++++++++++
3 files changed, 47 insertions(+), 1 deletion(-)
create mode 100644 bolt/test/RISCV/call-annotations.s
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index b815904f9672bcb..f48cf21852dcbef 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -170,6 +170,7 @@ class MCPlusBuilder {
/// MCPlusBuilder classes must use convert/lower/create* interfaces instead.
void setTailCall(MCInst &Inst);
+public:
/// Transfer annotations from \p SrcInst to \p DstInst.
void moveAnnotations(MCInst &&SrcInst, MCInst &DstInst) const {
assert(!getAnnotationInst(DstInst) &&
@@ -182,7 +183,6 @@ class MCPlusBuilder {
removeAnnotationInst(SrcInst);
}
-public:
/// Return iterator range covering def operands.
iterator_range<MCInst::iterator> defOperands(MCInst &Inst) const {
return make_range(Inst.begin(),
diff --git a/bolt/lib/Passes/FixRISCVCallsPass.cpp b/bolt/lib/Passes/FixRISCVCallsPass.cpp
index 34821ec80c1b4fd..9c3c74caea08ffe 100644
--- a/bolt/lib/Passes/FixRISCVCallsPass.cpp
+++ b/bolt/lib/Passes/FixRISCVCallsPass.cpp
@@ -19,6 +19,7 @@ void FixRISCVCallsPass::runOnFunction(BinaryFunction &BF) {
auto *Target = MIB->getTargetSymbol(*II);
assert(Target && "Cannot find call target");
+ auto OldCall = *II;
auto L = BC.scopeLock();
if (MIB->isTailCall(*II))
@@ -26,6 +27,7 @@ void FixRISCVCallsPass::runOnFunction(BinaryFunction &BF) {
else
MIB->createCall(*II, Target, Ctx);
+ MIB->moveAnnotations(std::move(OldCall), *II);
++II;
continue;
}
@@ -39,8 +41,19 @@ void FixRISCVCallsPass::runOnFunction(BinaryFunction &BF) {
auto *Target = MIB->getTargetSymbol(*II);
assert(Target && "Cannot find call target");
+ auto OldCall = *NextII;
auto L = BC.scopeLock();
MIB->createCall(*II, Target, Ctx);
+ MIB->moveAnnotations(std::move(OldCall), *II);
+
+ // The original offset was set on the jalr of the auipc+jalr pair. Since
+ // the whole pair is replaced by a call, adjust the offset by -4 (the
+ // size of a auipc).
+ if (auto Offset = MIB->getOffset(*II)) {
+ assert(*Offset >= 4 && "Illegal jalr offset");
+ MIB->setOffset(*II, *Offset - 4);
+ }
+
II = BB.eraseInstruction(NextII);
continue;
}
diff --git a/bolt/test/RISCV/call-annotations.s b/bolt/test/RISCV/call-annotations.s
new file mode 100644
index 000000000000000..e2c5a1040faee18
--- /dev/null
+++ b/bolt/test/RISCV/call-annotations.s
@@ -0,0 +1,33 @@
+/// Test that annotations are properly carried over to fixed calls.
+/// Note that --enable-bat is used to force offsets to be kept.
+
+// RUN: llvm-mc -triple riscv64 -filetype obj -o %t.o %s
+// RUN: ld.lld --emit-relocs -o %t %t.o
+// RUN: llvm-bolt --enable-bat --print-cfg --print-fix-riscv-calls \
+// RUN: --print-only=_start -o /dev/null %t | FileCheck %s
+
+ .text
+ .global f
+ .p2align 1
+f:
+ ret
+ .size f, .-f
+
+// CHECK-LABEL: Binary Function "_start" after building cfg {
+// CHECK: auipc ra, f
+// CHECK-NEXT: jalr ra, -4(ra) # Offset: 4
+// CHECK-NEXT: jal ra, f # Offset: 8
+// CHECK-NEXT: jal zero, f # TAILCALL # Offset: 12
+
+// CHECK-LABEL: Binary Function "_start" after fix-riscv-calls {
+// CHECK: call f # Offset: 0
+// CHECK-NEXT: call f # Offset: 8
+// CHECK-NEXT: tail f # TAILCALL # Offset: 12
+
+ .globl _start
+ .p2align 1
+_start:
+ call f
+ jal f
+ jal zero, f
+ .size _start, .-_start
>From 7b81499c0e725f765a887c3dfcaca2c398ccf849 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Thu, 21 Sep 2023 08:36:28 +0200
Subject: [PATCH 2/2] Expand `auto`s
---
bolt/lib/Passes/FixRISCVCallsPass.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/bolt/lib/Passes/FixRISCVCallsPass.cpp b/bolt/lib/Passes/FixRISCVCallsPass.cpp
index 9c3c74caea08ffe..963a8b04cf9db29 100644
--- a/bolt/lib/Passes/FixRISCVCallsPass.cpp
+++ b/bolt/lib/Passes/FixRISCVCallsPass.cpp
@@ -19,7 +19,7 @@ void FixRISCVCallsPass::runOnFunction(BinaryFunction &BF) {
auto *Target = MIB->getTargetSymbol(*II);
assert(Target && "Cannot find call target");
- auto OldCall = *II;
+ MCInst OldCall = *II;
auto L = BC.scopeLock();
if (MIB->isTailCall(*II))
@@ -41,7 +41,7 @@ void FixRISCVCallsPass::runOnFunction(BinaryFunction &BF) {
auto *Target = MIB->getTargetSymbol(*II);
assert(Target && "Cannot find call target");
- auto OldCall = *NextII;
+ MCInst OldCall = *NextII;
auto L = BC.scopeLock();
MIB->createCall(*II, Target, Ctx);
MIB->moveAnnotations(std::move(OldCall), *II);
@@ -49,7 +49,7 @@ void FixRISCVCallsPass::runOnFunction(BinaryFunction &BF) {
// The original offset was set on the jalr of the auipc+jalr pair. Since
// the whole pair is replaced by a call, adjust the offset by -4 (the
// size of a auipc).
- if (auto Offset = MIB->getOffset(*II)) {
+ if (std::optional<uint32_t> Offset = MIB->getOffset(*II)) {
assert(*Offset >= 4 && "Illegal jalr offset");
MIB->setOffset(*II, *Offset - 4);
}
More information about the llvm-commits
mailing list