[BOLT][RISCV] Fix AUIPC/JALR call rewriting - #216882
Open
rdtscp wants to merge 2 commits into
Open
Conversation
|
Hello @rdtscp 👋 Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.
Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description. Frequently asked questionsHow do I add reviewers? This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically. You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using What if there are no comments? If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers. Are any special GitHub settings required to contribute to LLVM? We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details. If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse. Thank you, |
rdtscp
marked this pull request as ready for review
August 18, 2026 15:30
rdtscp
requested review from
aaupov,
ayermolo,
maksfb,
paschalis-mpeis,
rafaelauler,
yavtuk and
yozhu
as code owners
August 18, 2026 15:30
|
@llvm/pr-subscribers-llvm-mc @llvm/pr-subscribers-backend-risc-v Author: Alexander Wilson (rdtscp) ChangesR_RISCV_CALL and R_RISCV_CALL_PLT cover an AUIPC/JALR pair, but BOLT treated them as four-byte relocations and decoded only the AUIPC immediate. Read both instructions and combine their signed high and low immediates so relocated call targets retain the low 12 bits. LTO can also leave linker-resolved intra-section AUIPC/JALR calls without relocations. Recognize valid standard call and tail-call pairs during disassembly, reconstruct the target (including JALR target-bit clearing), and attach a symbol reference before function reordering. This follows the RISC-V Unprivileged ISA RV32I sections "Integer Computational Instructions" (AUIPC) and "Control Transfer Instructions" (JALR): https://docs.riscv.org/reference/isa/v20260120/unpriv/rv32.html It also follows the RISC-V ELF psABI "Relocations" chapter, specifically "Procedure Calls", where R_RISCV_CALL and R_RISCV_CALL_PLT apply to the AUIPC/JALR pair: https://riscv-non-isa.github.io/riscv-elf-psabi-doc/#_relocations Tested with the focused RISC-V CoreTests and the full BOLT RISCV lit directory (38 passed). Scope: Relocation-less AUIPC/JALR recovery is intentionally RV64-only. RV32 XLEN-wrapped target arithmetic is deferred to a follow-up. Assisted-by: Codex Patch is 21.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/216882.diff 10 Files Affected:
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index be0d58af14fc4..25a0dde6dba34 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -913,6 +913,23 @@ class MCPlusBuilder {
return false;
}
+ /// Return true if \p First and \p Second form an AUIPC/JALR call pair with
+ /// linker-resolved immediates instead of a symbol reference. Such pairs can
+ /// be emitted for intra-section calls and need to be resymbolized before the
+ /// caller is moved.
+ virtual bool isUnsymbolizedRISCVCall(const MCInst &First,
+ const MCInst &Second) const {
+ return false;
+ }
+
+ /// Return the byte offset from AUIPC to the target of an unsymbolized
+ /// AUIPC/JALR call pair, including JALR's clearing of target bit zero.
+ virtual int64_t getUnsymbolizedRISCVCallOffset(const MCInst &First,
+ const MCInst &Second) const {
+ llvm_unreachable("not implemented");
+ return 0;
+ }
+
/// Used to fill the executable space with instructions
/// that will trap.
virtual StringRef getTrapFillValue() const {
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index a81fa2f45c206..d668dfc95c827 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -24,6 +24,7 @@
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/BinaryFormat/ELF.h"
#include "llvm/Demangle/Demangle.h"
#include "llvm/MC/MCAsmInfo.h"
#include "llvm/MC/MCContext.h"
@@ -1421,6 +1422,35 @@ Error BinaryFunction::disassemble() {
if (IsUnsupported)
setIgnored();
+ // Recover linker-resolved intra-section calls without relocations. The
+ // current instruction is the JALR and the AUIPC is four bytes earlier in
+ // the instruction map. Resymbolizing the AUIPC lets BOLT update the pair
+ // if function reordering moves the caller relative to the callee. Limit
+ // this recovery to RV64 until RV32 target calculation handles XLEN wrap.
+ if (BC.TheTriple->isRISCV64() && Offset >= 4) {
+ auto PrevII = Instructions.find(Offset - 4);
+ if (PrevII != Instructions.end() &&
+ BC.MIB->isUnsymbolizedRISCVCall(PrevII->second, Instruction)) {
+ const uint64_t Target =
+ AbsoluteInstrAddr - 4 +
+ BC.MIB->getUnsymbolizedRISCVCallOffset(PrevII->second, Instruction);
+ if (BinaryFunction *TargetBF =
+ BC.getBinaryFunctionContainingAddress(Target)) {
+ BC.addInterproceduralReference(this, Target);
+ MCSymbol *TargetSymbol =
+ BC.handleExternalBranchTarget(Target, *this, *TargetBF);
+ if (TargetSymbol) {
+ int64_t Value = 0;
+ const bool Replaced = BC.MIB->replaceImmWithSymbolRef(
+ PrevII->second, TargetSymbol, /*Addend=*/0, Ctx.get(), Value,
+ ELF::R_RISCV_CALL_PLT);
+ (void)Replaced;
+ assert(Replaced && "cannot symbolize RISC-V call");
+ }
+ }
+ }
+ }
+
if (MIB->isBranch(Instruction) || MIB->isCall(Instruction)) {
uint64_t TargetAddress = 0;
if (MIB->evaluateBranch(Instruction, AbsoluteInstrAddr, Size,
diff --git a/bolt/lib/Core/Relocation.cpp b/bolt/lib/Core/Relocation.cpp
index 55d5e07042897..2d19b24c8c05a 100644
--- a/bolt/lib/Core/Relocation.cpp
+++ b/bolt/lib/Core/Relocation.cpp
@@ -229,8 +229,6 @@ static size_t getSizeForTypeRISCV(uint32_t Type) {
case ELF::R_RISCV_PCREL_LO12_I:
case ELF::R_RISCV_PCREL_LO12_S:
case ELF::R_RISCV_32_PCREL:
- case ELF::R_RISCV_CALL:
- case ELF::R_RISCV_CALL_PLT:
case ELF::R_RISCV_ADD32:
case ELF::R_RISCV_SUB32:
case ELF::R_RISCV_HI20:
@@ -239,6 +237,8 @@ static size_t getSizeForTypeRISCV(uint32_t Type) {
case ELF::R_RISCV_32:
return 4;
case ELF::R_RISCV_64:
+ case ELF::R_RISCV_CALL:
+ case ELF::R_RISCV_CALL_PLT:
case ELF::R_RISCV_GOT_HI20:
case ELF::R_RISCV_TLS_GOT_HI20:
case ELF::R_RISCV_TLS_GD_HI20:
@@ -502,7 +502,11 @@ static uint64_t extractValueRISCV(uint32_t Type, uint64_t Contents,
return extractJImmRISCV(Contents);
case ELF::R_RISCV_CALL:
case ELF::R_RISCV_CALL_PLT:
- return extractUImmRISCV(Contents);
+ // The psABI "Relocations" chapter's "Procedure Calls" section defines
+ // R_RISCV_CALL and R_RISCV_CALL_PLT over an AUIPC/JALR pair. Decode both
+ // instructions so the addend includes the low 12 bits carried by JALR.
+ return extractUImmRISCV(Contents & 0xffffffff) +
+ extractIImmRISCV(Contents >> 32);
case ELF::R_RISCV_BRANCH:
return extractBImmRISCV(Contents);
case ELF::R_RISCV_GOT_HI20:
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 1511e4744124a..c6579c76bcd32 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -19,6 +19,7 @@
#include "llvm/MC/MCInstBuilder.h"
#include "llvm/MC/MCSubtargetInfo.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/MathExtras.h"
#define DEBUG_TYPE "mcplus"
@@ -27,6 +28,33 @@ using namespace bolt;
namespace {
+bool isValidUnsymbolizedCallAUIPC(const MCInst &Inst) {
+ if (Inst.getOpcode() != RISCV::AUIPC ||
+ MCPlus::getNumPrimeOperands(Inst) != 2)
+ return false;
+
+ const MCOperand &Destination = Inst.getOperand(0);
+ return Destination.isReg() && Destination.getReg() != RISCV::X0 &&
+ Inst.getOperand(1).isImm();
+}
+
+bool isValidUnsymbolizedCallJALR(const MCInst &Inst) {
+ if (Inst.getOpcode() != RISCV::JALR || MCPlus::getNumPrimeOperands(Inst) != 3)
+ return false;
+
+ return Inst.getOperand(0).isReg() && Inst.getOperand(1).isReg() &&
+ Inst.getOperand(2).isImm();
+}
+
+bool hasSupportedCallRegisters(const MCInst &First, const MCInst &Second) {
+ const MCPhysReg Base = First.getOperand(0).getReg();
+ if (Second.getOperand(1).getReg() != Base)
+ return false;
+
+ const MCPhysReg Link = Second.getOperand(0).getReg();
+ return Link == RISCV::X0 || Link == Base;
+}
+
class RISCVMCPlusBuilder : public MCPlusBuilder {
bool isRV64() const { return STI->hasFeature(RISCV::Feature64Bit); }
unsigned regSize() const { return isRV64() ? 8 : 4; }
@@ -268,7 +296,20 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
void createCall(MCInst &Inst, const MCSymbol *Target,
MCContext *Ctx) override {
- return createCall(RISCV::PseudoCALL, Inst, Target, Ctx);
+ MCPhysReg Link = RISCV::X1;
+ if ((Inst.getOpcode() == RISCV::JAL || Inst.getOpcode() == RISCV::JALR ||
+ Inst.getOpcode() == RISCV::PseudoCALLReg) &&
+ Inst.getNumOperands() && Inst.getOperand(0).isReg())
+ Link = Inst.getOperand(0).getReg();
+
+ if (Link == RISCV::X1)
+ return createCall(RISCV::PseudoCALL, Inst, Target, Ctx);
+
+ Inst.setOpcode(RISCV::PseudoCALLReg);
+ Inst.clear();
+ Inst.addOperand(MCOperand::createReg(Link));
+ Inst.addOperand(MCOperand::createExpr(MCSpecifierExpr::create(
+ MCSymbolRefExpr::create(Target, *Ctx), RISCV::S_CALL_PLT, *Ctx)));
}
void createLongTailCall(InstructionListType &Seq, const MCSymbol *Target,
@@ -476,7 +517,9 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
}
bool isCallAuipc(const MCInst &Inst) const {
- if (Inst.getOpcode() != RISCV::AUIPC)
+ if (Inst.getOpcode() != RISCV::AUIPC ||
+ MCPlus::getNumPrimeOperands(Inst) != 2 || !Inst.getOperand(0).isReg() ||
+ Inst.getOperand(0).getReg() == RISCV::X0)
return false;
const auto &ImmOp = Inst.getOperand(1);
@@ -497,11 +540,31 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
}
bool isRISCVCall(const MCInst &First, const MCInst &Second) const override {
- if (!isCallAuipc(First))
+ if (!isCallAuipc(First) || !isValidUnsymbolizedCallJALR(Second))
return false;
- assert(Second.getOpcode() == RISCV::JALR);
- return true;
+ return hasSupportedCallRegisters(First, Second);
+ }
+
+ bool isUnsymbolizedRISCVCall(const MCInst &First,
+ const MCInst &Second) const override {
+ if (!isValidUnsymbolizedCallAUIPC(First) ||
+ !isValidUnsymbolizedCallJALR(Second))
+ return false;
+
+ return hasSupportedCallRegisters(First, Second);
+ }
+
+ int64_t getUnsymbolizedRISCVCallOffset(const MCInst &First,
+ const MCInst &Second) const override {
+ // The RV32I "Integer Computational Instructions" section defines AUIPC's
+ // offset as the sign-extended 20-bit U-immediate shifted left by 12. The
+ // "Control Transfer Instructions" section defines JALR as adding its
+ // sign-extended 12-bit I-immediate and clearing target bit zero. Mask the
+ // decoded AUIPC operand back to its encoded field before sign extension.
+ const int64_t Hi = SignExtend64<32>(
+ (static_cast<uint64_t>(First.getOperand(1).getImm()) & 0xfffff) << 12);
+ return (Hi + Second.getOperand(2).getImm()) & ~1LL;
}
uint16_t getMinFunctionAlignment() const override {
diff --git a/bolt/test/RISCV/Inputs/unsymbolized-call-order.txt b/bolt/test/RISCV/Inputs/unsymbolized-call-order.txt
new file mode 100644
index 0000000000000..801f0aa6638b9
--- /dev/null
+++ b/bolt/test/RISCV/Inputs/unsymbolized-call-order.txt
@@ -0,0 +1,3 @@
+target
+relocated_call
+_start
diff --git a/bolt/test/RISCV/call-relocation-pair.s b/bolt/test/RISCV/call-relocation-pair.s
new file mode 100644
index 0000000000000..aa8919ac24d77
--- /dev/null
+++ b/bolt/test/RISCV/call-relocation-pair.s
@@ -0,0 +1,76 @@
+// Test that R_RISCV_CALL and R_RISCV_CALL_PLT cover and decode the complete
+// AUIPC/JALR instruction pair.
+
+// RUN: llvm-mc -triple riscv64 -mattr=-relax -filetype=obj -o %t.o %s
+// RUN: ld.lld --no-relax --emit-relocs -o %t %t.o
+// RUN: llvm-readelf --relocations %t | FileCheck --check-prefix=RELOCS %s
+// RUN: llvm-bolt --print-fix-riscv-calls --print-only=_start -o %t.bolt %t \
+// RUN: | FileCheck --check-prefix=BOLT %s
+// RUN: llvm-objdump -d %t.bolt | FileCheck --check-prefix=OBJDUMP %s
+
+// RELOCS: R_RISCV_CALL {{.*}} target_call
+// RELOCS: R_RISCV_CALL_PLT {{.*}} target_call_plt
+// RELOCS: R_RISCV_CALL_PLT {{.*}} target_call_t0
+
+// BOLT-LABEL: Binary Function "_start" after fix-riscv-calls {
+// BOLT: nop
+// BOLT-NEXT: call target_call
+// BOLT-NEXT: nop
+// BOLT-NEXT: call target_call_plt
+// BOLT-NEXT: nop
+// BOLT-NEXT: call t0, target_call_t0
+
+// OBJDUMP-LABEL: <_start>:
+// OBJDUMP: nop
+// OBJDUMP-NEXT: auipc ra,
+// OBJDUMP-NEXT: jalr {{.*}}(ra)
+// OBJDUMP-NEXT: nop
+// OBJDUMP-NEXT: auipc ra,
+// OBJDUMP-NEXT: jalr {{.*}}(ra)
+// OBJDUMP-NEXT: nop
+// OBJDUMP-NEXT: auipc t0,
+// OBJDUMP-NEXT: jalr t0, {{.*}}(t0)
+// OBJDUMP-LABEL: <target_call>:
+// OBJDUMP-LABEL: <target_call_plt>:
+// OBJDUMP-LABEL: <target_call_t0>:
+
+ .text
+ .option norvc
+ .option norelax
+
+ .globl _start
+ .type _start,@function
+_start:
+ .reloc ., R_RISCV_CALL, target_call
+ auipc ra, 0
+ jalr ra
+ .reloc ., R_RISCV_CALL_PLT, target_call_plt
+ auipc ra, 0
+ jalr ra
+ .reloc ., R_RISCV_CALL_PLT, target_call_t0
+ auipc t0, 0
+ jalr t0, 0(t0)
+ ret
+ .size _start, .-_start
+
+ .skip (1 << 21) + 0x7c
+
+ .globl target_call
+ .type target_call,@function
+target_call:
+ ret
+ .size target_call, .-target_call
+
+ .skip 0x84
+
+ .globl target_call_plt
+ .type target_call_plt,@function
+target_call_plt:
+ ret
+ .size target_call_plt, .-target_call_plt
+
+ .globl target_call_t0
+ .type target_call_t0,@function
+target_call_t0:
+ ret
+ .size target_call_t0, .-target_call_t0
diff --git a/bolt/test/RISCV/unsymbolized-call-entry.s b/bolt/test/RISCV/unsymbolized-call-entry.s
new file mode 100644
index 0000000000000..abd51a9f9d385
--- /dev/null
+++ b/bolt/test/RISCV/unsymbolized-call-entry.s
@@ -0,0 +1,57 @@
+// Test recovery of an RV64 linker-resolved call using the alternate link
+// register and targeting an entry point inside a function.
+
+// RUN: llvm-mc -triple riscv64 -mattr=-relax -filetype=obj -o %t.o %s
+// RUN: ld.lld --no-relax --emit-relocs -o %t %t.o
+// RUN: llvm-objdump -dr %t | FileCheck --check-prefix=INPUT %s
+// RUN: llvm-bolt --print-cfg --print-fix-riscv-calls --print-only=_start \
+// RUN: --reorder-functions=user \
+// RUN: --function-order=%p/Inputs/unsymbolized-call-order.txt \
+// RUN: -o %t.bolt %t | FileCheck --check-prefix=BOLT %s
+// RUN: llvm-objdump -d %t.bolt | FileCheck --check-prefix=OBJDUMP %s
+
+// INPUT-LABEL: <_start>:
+// INPUT: auipc t0, 0x200
+// INPUT-NEXT: jalr t0, 0x8c(t0) <target_entry>
+
+// BOLT-LABEL: Binary Function "_start" after building cfg {
+// BOLT: auipc t0, {{.*}}target_entry{{.*}}
+// BOLT-NEXT: jalr t0, {{.*}}(t0)
+// BOLT-LABEL: Binary Function "_start" after fix-riscv-calls {
+// BOLT: call t0, {{.*}}target_entry{{.*}}
+
+// OBJDUMP-LABEL: <target>:
+// OBJDUMP: addi a0, a0, {{(0x)?1}}
+// OBJDUMP-LABEL: <target_entry>:
+// OBJDUMP: ret
+// OBJDUMP-LABEL: <_start>:
+// OBJDUMP: jal t0, {{.*}} <target_entry>
+
+ .text
+ .option norvc
+ .option norelax
+
+ .globl _start
+ .type _start,@function
+_start:
+ auipc t0, 0x200
+ jalr t0, 0x8c(t0)
+ ret
+ .size _start, .-_start
+
+ .skip (1 << 21) + 0x7c
+
+ .globl target
+ .type target,@function
+target:
+ addi a0, a0, 1
+target_entry:
+ ret
+ .size target, .-target
+
+ .globl relocated_call
+ .type relocated_call,@function
+relocated_call:
+ call target
+ ret
+ .size relocated_call, .-relocated_call
diff --git a/bolt/test/RISCV/unsymbolized-call.s b/bolt/test/RISCV/unsymbolized-call.s
new file mode 100644
index 0000000000000..d9fb765b22e7f
--- /dev/null
+++ b/bolt/test/RISCV/unsymbolized-call.s
@@ -0,0 +1,57 @@
+// Test recovery of a linker-resolved AUIPC/JALR pair that has no relocation,
+// even though the rest of the executable retains relocations.
+
+// RUN: llvm-mc -triple riscv64 -mattr=-relax -filetype=obj -o %t.o %s
+// RUN: ld.lld --no-relax --emit-relocs -o %t %t.o
+// RUN: llvm-objdump -dr %t | FileCheck --check-prefix=INPUT %s
+// RUN: llvm-bolt --print-cfg --print-fix-riscv-calls --print-only=_start \
+// RUN: --reorder-functions=user \
+// RUN: --function-order=%p/Inputs/unsymbolized-call-order.txt \
+// RUN: -o %t.bolt %t | FileCheck --check-prefix=BOLT %s
+// RUN: llvm-objdump -d %t.bolt | FileCheck --check-prefix=OBJDUMP %s
+
+// INPUT-LABEL: <_start>:
+// INPUT: auipc ra, 0x200
+// INPUT-NEXT: jalr 0x88(ra) <target>
+// INPUT-NEXT: ret
+// INPUT-LABEL: <relocated_call>:
+// INPUT: R_RISCV_CALL_PLT target
+
+// BOLT-LABEL: Binary Function "_start" after building cfg {
+// BOLT: auipc ra, target
+// BOLT-NEXT: jalr {{.*}}(ra)
+// BOLT-LABEL: Binary Function "_start" after fix-riscv-calls {
+// BOLT: call target
+
+// OBJDUMP-LABEL: <target>:
+// OBJDUMP-LABEL: <_start>:
+// OBJDUMP: jal {{.*}} <target>
+
+ .text
+ .option norvc
+ .option norelax
+
+ .globl _start
+ .type _start,@function
+_start:
+ // The target starts 0x200088 bytes after this AUIPC. Spell out the resolved
+ // immediates so this pair has no relocation, as happens after LTO linking.
+ auipc ra, 0x200
+ jalr ra, 0x88(ra)
+ ret
+ .size _start, .-_start
+
+ .skip (1 << 21) + 0x7c
+
+ .globl target
+ .type target,@function
+target:
+ ret
+ .size target, .-target
+
+ .globl relocated_call
+ .type relocated_call,@function
+relocated_call:
+ call target
+ ret
+ .size relocated_call, .-relocated_call
diff --git a/bolt/unittests/Core/CMakeLists.txt b/bolt/unittests/Core/CMakeLists.txt
index 297dec7449202..b4e0e95acff1b 100644
--- a/bolt/unittests/Core/CMakeLists.txt
+++ b/bolt/unittests/Core/CMakeLists.txt
@@ -11,6 +11,7 @@ add_bolt_unittest(CoreTests
MCPlusBuilder.cpp
MemoryMaps.cpp
DynoStats.cpp
+ RISCVMCPlusBuilder.cpp
# FIXME CoreTests uses `llvm::detail::TakeError(llvm::Error)`, but linking
# to LLVMTestingSupport introduces a transitive dependency on the
diff --git a/bolt/unittests/Core/RISCVMCPlusBuilder.cpp b/bolt/unittests/Core/RISCVMCPlusBuilder.cpp
new file mode 100644
index 0000000000000..8213faf1612b8
--- /dev/null
+++ b/bolt/unittests/Core/RISCVMCPlusBuilder.cpp
@@ -0,0 +1,145 @@
+//===- bolt/unittest/Core/RISCVMCPlusBuilder.cpp --------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifdef RISCV_AVAILABLE
+
+#include "MCTargetDesc/RISCVMCTargetDesc.h"
+#include "bolt/Core/BinaryContext.h"
+#include "bolt/Rewrite/RewriteInstance.h"
+#include "llvm/BinaryFormat/ELF.h"
+#include "llvm/DebugInfo/DWARF/DWARFContext.h"
+#include "llvm/MC/MCInstBuilder.h"
+#include "llvm/Support/TargetSelect.h"
+#include "llvm/TargetParser/SubtargetFeature.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::bolt;
+using namespace llvm::object;
+
+namespace {
+
+class RISCVMCPlusBuilderTest : public testing::Test {
+protected:
+ void SetUp() override {
+ LLVMInitializeRISCVTargetInfo();
+ LLVMInitializeRISCVTargetMC();
+ LLVMInitializeRISCVDisassembler();
+
+ memcpy(ElfBuf, "\177ELF", 4);
+ auto *EHdr = reinterpret_cast<ELF64LE::Ehdr *>(ElfBuf);
+ EHdr->e_ident[ELF::EI_CLASS] = ELF::ELFCLASS64;
+ EHdr->e_ident[ELF::EI_DATA] = ELF::ELFDATA2LSB;
+ EHdr->e_machine = ELF::EM_RISCV;
+ MemoryBufferRef Source(StringRef(ElfBuf, sizeof(ElfBuf)), "ELF");
+ ObjFile = cantFail(ObjectFile::createObjectFile(Source));
+
+ Relocation::Arch = Triple::riscv64;
+ SubtargetFeatures Features("+m,+a,+f,+d,+c");
+ BC = cantFail(BinaryContext::createBinaryContext(
+ ObjFile->makeTriple(), std::make_shared<orc::SymbolStringPool>(),
+ ObjFile->getFileName(), &Features, true, DWARFContext::create(*ObjFile),
+ {llvm::outs(), llvm::errs()}));
+ BC->initializeTarget(std::unique_ptr<MCPlusBuilder>(
+ createMCPlusBuilder(Triple::riscv64, BC->MIA.get(), BC->MII.get(),
+ BC->MRI.get(), BC->STI.get())));
+ }
+
+ static MCInst makeAUIPC(MCPhysReg Destination, int64_t Immediate) {
+ return MCInstBuilder(RISCV::AUIPC).addReg(Destination).addImm(Immediate);
+ }
+
+ static MCInst makeJALR(MCPhysReg Link, MCPhysReg Base, int64_t Immediate) {
+ return MCInstBuilder(RISCV::JALR)
+ .addReg(Link)
+ .addReg(Base)
+ .addImm(Immediate);
+ }
+
+ char ElfBuf[sizeof(ELF64LE::Ehdr)] = {};
+ std::unique_ptr<ObjectFile> ObjFile;
+ std::unique_ptr<BinaryContext> BC;
+};
+
+TEST_F(RISCVMCPlusBuilderTest,
+ UnsymbolizedCall_StandardCallAndTailCall_AreRecognized) {
+ const MCInst CallAUIPC = makeAUIPC(RISCV::X1, 1);
+ const MCInst CallJALR = makeJALR(RISCV::X1, RISCV::X1, -2048);
+ const MCInst OddCallJALR = makeJALR(RISCV::X1, RISCV::X1, -2047);
+ EXPECT_TRUE(BC->MIB->isUnsymbolizedRISCVCall(CallAUIPC, CallJALR));
+ EXPECT_EQ(2048, BC->MIB->getUnsymbolizedRISCVCallOffset(CallAUIPC, CallJALR));
+ EXPECT_EQ(2048,
+ BC->MIB->getUnsymbolizedRISCVCallOffset(CallAUIPC, OddCallJALR));
+
+ const MCInst TailAUIPC = makeAUIPC(RISCV::X6, 0xfffff);
+ const MCInst TailJALR = makeJALR(RISCV::X0, RISCV::X6, -4);
+ EXPECT_TRUE(BC->MIB->isUnsymbolizedRISCVCall(TailAUIPC, TailJALR));
+ EXPECT_EQ(-4100,
+ BC->MIB->getUnsymbolizedRISCVCallOffset(TailAUIPC, TailJALR));
+
+ const MCInst AlternateLinkAUIPC = makeAUIPC(RISCV::X5, 1);
+ const MCInst AlternateLinkJALR = makeJALR(RISCV::X5, RISCV::X5, 0);
+ EXPECT_TRUE(
+ BC->MIB->isUnsymbolizedRISCVCall(AlternateLinkAUIPC, AlternateLinkJALR));
+}
+
+TEST_F(RISCVMCPlusBuilderTest, UnsymbolizedCall_NearMatches_AreRejected) {
+ const MCInst AUIPC = makeAUIPC(RISCV::X1, 1);
+ const MCInst JALR = makeJALR(RISCV::X1, RISCV::X1, 0);
+
+ EXPECT_FALSE(BC->MIB->isUnsymbolizedRISCVCall(
+ MCInstBuilder(RISCV::LUI).addReg(RISCV::X1).addImm(1), JALR));
+ EXPECT_FALSE(BC->MIB->isUnsymbolizedRISCVCall(
+ AUIPC, MCInstBuilder(RISCV::JAL).addReg(RISCV::X1).addImm(0)))...
[truncated]
|
rafaelauler
reviewed
Aug 18, 2026
rafaelauler
left a comment
rafaelauler
left a comment
Contributor
There was a problem hiding this comment.
Regarding the extra stuff being added into the disassembly loop, I want to replicate my comment at #149658:
On the other hand, I should warn that the kind of stuff you're doing here is what a BOLT target symbolizer would be doing (sanitizing the references based on relocations). Take a look at bolt/lib/Target/X86/X86MCSymbolizer.cpp and bolt/lib/Target/AArch64/AArch64MCSymbolizer.cpp for references on other targets. I noticed RISCV has no symbolizer yet -- the RISCV port probably needs to implement one, otherwise you will be adding a lot of logic to the body of ::disassemble, which was designed to operate with the symbolizer.
I don't know if it's possible to solve the RISCV issues by refactoring logic to live in the symbolizer that works with the disassembler, though, but if it is possible, it's something to consider to make the port more similar to the other two archs (x86 and aarch64).
Regarding unittesting: try to steer AI away from unittests as we do prefer the LIT ones, if possible.
rdtscp
marked this pull request as draft
August 20, 2026 00:14
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
Contributor
|
The LLVM RISC-V backend and BOLT changes need to be split. Maybe PR 217550 should be enough. |
Contributor
Author
|
Thank you @Thrrreeee for the review! Let me read your work and figure out what can be removed here. I'll try address all feedback at once in my next commit 🙏 |
topperc
self-requested a review
August 20, 2026 18:49
topperc
reviewed
Aug 20, 2026
| LLVMSymbolLookupCallback SymbolLookUp, | ||
| void *DisInfo, MCContext *Ctx, | ||
| std::unique_ptr<MCRelocationInfo> &&RelInfo) { | ||
| return new RISCVExternalSymbolizer(*Ctx, std::move(RelInfo), GetOpInfo, |
rdtscp
marked this pull request as draft
August 20, 2026 21:17
rdtscp
force-pushed
the
risc-v-bolt
branch
2 times, most recently
from
August 20, 2026 21:47
c34d5e8 to
b2b980e
Compare
kito-cheng
reviewed
Aug 21, 2026
kito-cheng
left a comment
kito-cheng
left a comment
Member
There was a problem hiding this comment.
I know there is bug in GOT decoding, but I would incline it should be a independent PR instead of mixing in this PR, the GOT issue is more complicate than the AUIPC/JALR, function call guarantee hi-part low-part will appear together but GOT load isn't.
Also does it possible to split symbolizer part into a separate PR as well?
| .addImm((Imm >> 12) & 0xFFFFF)); | ||
| Insts.emplace_back( | ||
| MCInstBuilder(RISCV::LUI).addReg(RISCV::X6).addImm((Imm)&0xFFF)); | ||
| MCInstBuilder(RISCV::LUI).addReg(RISCV::X6).addImm((Imm) & 0xFFF)); |
Member
There was a problem hiding this comment.
Keep it unchanged since, it seems just reformat, it could be a small separate NFC patch
Contributor
cc @kito-cheng , this is now handled in the separate BOLT PR #217944 The implementation does not assume that the GOT load immediately follows the AUIPC. It pre-scans R_RISCV_PCREL_LO12_{I,S} relocations and associates each low relocation with the referenced high instruction, so unrelated instructions between the pair and basic-block reordering are handled. The new RV32 and RV64 tests cover these cases. There may still be other cases that are not fully supported and will need to be addressed in follow-up fixes. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
R_RISCV_CALL and R_RISCV_CALL_PLT cover an AUIPC/JALR pair. Read both instructions and combine their signed high and low immediates so relocated call targets retain the low 12 bits. Also recover RV64 linker-resolved intra-section call and tail-call pairs without relocations, including alternate link registers. Relocation-less recovery remains intentionally RV64-only. This revision addresses review feedback by building on the generic RISC-V symbolizer and GOT handling from llvm#217944 and retaining only call-specific changes here. This follows the RISC-V Unprivileged ISA sections on AUIPC and JALR: https://docs.riscv.org/reference/isa/v20260120/unpriv/rv32.html It also follows the RISC-V ELF psABI procedure-call relocations: https://riscv-non-isa.github.io/riscv-elf-psabi-doc/#_relocations Assisted-by: Codex
rdtscp
marked this pull request as ready for review
August 21, 2026 22:34
Contributor
Author
|
Thanks @Thrrreeee , @kito-cheng, and @topperc for the review! I see #217550 is merged, and I have rebased this on top of #217944 now |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #217550 for the RISC-V HI20/LO12 symbolic-disassembly hooks. This branch is based directly on the head of #217550; the commits owned by this PR modify only
bolt/.R_RISCV_CALLandR_RISCV_CALL_PLTcover an AUIPC/JALR pair, but BOLT treated them as four-byte relocations and decoded only the AUIPC immediate. Read both instructions and combine their signed high and low immediates so relocated call targets retain the low 12 bits.Move RISC-V instruction-relocation handling into
RISCVMCSymbolizer, following the existing X86 and AArch64 target-symbolizer architecture. The symbolizer handles CALL, GOT, TLS, and PC-relative high/low relocations while pairing%pcrel_lousers with their referenced%pcrel_hiinstruction instead of assuming the instructions are adjacent.LTO can also leave linker-resolved intra-section AUIPC/JALR calls without relocations. On RV64, recognize valid call and tail-call pairs during disassembly, reconstruct the target (including JALR target-bit clearing), and attach an exact entry-point symbol before function reordering. Preserve alternate link registers such as
x5when canonicalizing calls.This follows the RISC-V Unprivileged ISA RV32I sections "Integer Computational Instructions" (AUIPC) and "Control Transfer Instructions" (JALR):
https://docs.riscv.org/reference/isa/v20260120/unpriv/rv32.html
It also follows the RISC-V ELF psABI "Relocations" chapter, including "Procedure Calls" and the PC-relative HI20/LO12 pairing rules:
https://riscv-non-isa.github.io/riscv-elf-psabi-doc/#_relocations
Tests include relocation-backed calls, relocation-less RV64 calls and tail calls, alternate link registers, non-adjacent GOT and PC-relative relocation pairs, and RV32/RV64 relocation preservation.
Validation:
llvm-bolt,llvm-mc,llvm-objdump,llvm-readelf,lld, andllvm-otoolafter stacking this PR on [RISCV][Disassembler] Symbolize UImm20 and SImm12Lo operands #217550.bolt/test/RISCVandllvm/test/MC/RISCV: 652/652 tests passed.Scope: Inferring relocation-less AUIPC/JALR pairs is intentionally RV64-only. RV32 XLEN-wrapped target arithmetic remains out of scope. Relocation-backed RV32 behavior continues to be supported.
Assisted-by: Codex