Skip to content

[RISCV] Support --symbolize-operands #166656

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+52 −2
Open

[RISCV] Support --symbolize-operands #166656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion llvm/docs/CommandGuide/llvm-objdump.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ OPTIONS
any analysis with a special representation (i.e. BlockFrequency,
BranchProbability, etc) are printed as raw hex values.

Only supported for AArch64, BPF, PowerPC, and X86.
Only supported for AArch64, BPF, PowerPC, RISC-V, and X86.

Example:
A non-symbolized branch instruction with a local target and pc-relative memory access like
Expand Down
1 change: 1 addition & 0 deletions llvm/docs/ReleaseNotes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ Changes to the RISC-V Backend
* Adds experimental support for the 'Zibi` (Branch with Immediate) extension.
* Add support for Zvfofp8min (OFP8 conversion extension)
* Adds assembler support for the Andes `XAndesvsinth` (Andes Vector Small Int Handling Extension).
* `llvm-objdump` now has support for `--symbolize-operands` with RISC-V.

Changes to the WebAssembly Backend
----------------------------------
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "llvm/MC/MCExpr.h"
#include "llvm/MC/MCInst.h"
#include "llvm/MC/MCInstPrinter.h"
#include "llvm/MC/MCInstrAnalysis.h"
#include "llvm/MC/MCSubtargetInfo.h"
#include "llvm/MC/MCSymbol.h"
#include "llvm/Support/CommandLine.h"
Expand Down Expand Up @@ -108,6 +109,10 @@ void RISCVInstPrinter::printBranchOperand(const MCInst *MI, uint64_t Address,
unsigned OpNo,
const MCSubtargetInfo &STI,
raw_ostream &O) {
// Do not print the numeric target address when symbolizing.
if (SymbolizeOperands)
return;

const MCOperand &MO = MI->getOperand(OpNo);
if (!MO.isImm())
return printOperand(MI, OpNo, STI, O);
Expand Down
43 changes: 43 additions & 0 deletions llvm/test/MC/RISCV/symbolize-operands.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# RUN: llvm-mc -triple=riscv32 < %s -mattr=-relax -filetype=obj -o - \
# RUN: | llvm-objdump -d --no-leading-addr --no-show-raw-insn --symbolize-operands - \
# RUN: | FileCheck %s

# CHECK-LABEL: <.text>:
.text
.p2align 2
# CHECK: blez a0, <L3>
Copy link
Member

@arichardson arichardson Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this currently only support branches or also instructions like jalr/ld?

Would be good to add a test showing how those behave as well. And maybe one test case where symbolization fails? something like blez a0, 0x123?

Copy link
Member

@arichardson arichardson Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the diff below it looks like this only affects branches, but I think it might still be good to have something like

foo:
   j bar
bar:
   ...

in the test?

Copy link
Member Author

@lenary lenary Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't yet have support for addi/loads/stores, as far as I understand. That is in #144620.

I will look at the cases where symbolization might fail, but I'm not sure they're covered well in other architectures either. LLVM would interpret blez a0, 0x122 (0x123 is not encodable) as a branch to pc+0x122, which it might still create a symbol for even if it's not dumped for that function, so I don't know what the answer is there.

I will try some cases, but I think the "don't print the number" signal doesn't depend on whether something symbolized correctly, which might be good or bad.

I also don't mind if we print both the number and the symbol - it does make it harder to diff the output, but post-processing is normally needed before a dump can be diffed well anyway.

blez a0, .LBB0_6
li a3, 0
li a2, 0
# CHECK: j <L1>
j .LBB0_3
Comment on lines +12 to +13
Copy link
Member Author

@lenary lenary Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arichardson this includes a j for jumps?

Copy link
Member

@arichardson arichardson Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I missed that one, so looks like we do handle jalr (at least for local symbols?). Could add another one calling a non-local symbol?

# CHECK-NEXT: <L0>:
.LBB0_2:
addi a3, a3, 1
# CHECK: beq a3, a0, <L4>
beq a3, a0, .LBB0_7
# CHECK-NEXT: <L1>:
.LBB0_3:
slli a4, a3, 2
add a4, a1, a4
lw a5, 0(a4)
lbu a4, 0(a5)
# CHECK: beqz a4, <L0>
beqz a4, .LBB0_2
addi a5, a5, 1
# CHECK: <L2>
.LBB0_5:
add a2, a2, a4
lbu a4, 0(a5)
addi a5, a5, 1
# CHECK: bnez a4, <L2>
bnez a4, .LBB0_5
# CHECK-NEXT: j <L0>
j .LBB0_2
# CHECK-NEXT: <L3>:
.LBB0_6:
li a2, 0
# CHECK: <L4>:
.LBB0_7:
mv a0, a2
ret
3 changes: 2 additions & 1 deletion llvm/tools/llvm-objdump/llvm-objdump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,8 @@ collectLocalBranchTargets(ArrayRef<uint8_t> Bytes, MCInstrAnalysis *MIA,
const bool isX86 = STI->getTargetTriple().isX86();
const bool isAArch64 = STI->getTargetTriple().isAArch64();
const bool isBPF = STI->getTargetTriple().isBPF();
if (!isPPC && !isX86 && !isAArch64 && !isBPF)
const bool isRISCV = STI->getTargetTriple().isRISCV();
if (!isPPC && !isX86 && !isAArch64 && !isBPF && !isRISCV)
return;

if (MIA)
Expand Down