Skip to content

riscv_dis_opts_minor_1

Jump to bottom
Tsukasa OI edited this page Jul 16, 2022 · 23 revisions

Disassembler: Minor optimizations (batch 1)

Requires

Feature Description

This patchset intends to organize the core disassembler for minor optimizations (contained in this patchset), future optimizations with bigger performance improvements and clarity.

There are some big performance improvements on certain circumstances but they are rarely significant in the real world benchmark.

It will be a prerequisite of upcoming core disassembler changes:

1a. New state initialization functions with minimized calls (PATCH 1/7)

Current disassembler repeatedly checks current state per instruction:

  • Whether riscv_gpr_names is initialized to a non-NULL value
  • Whether the Zfinx extension is available

... but they are not frequently changed.

Calling riscv_subset_supports repeatedly harms the performance in a measurable way (3-13% in total) and this patchset reduces this Zfinx query call from per-instruction to per-{arch,option} change.

Note that only a small part of per-instruction calls to riscv_subset_supports are taken care of and this change alone wouldn't measurably improve the performance (I will submit another batch of real optimizations later).

riscv_gpr_names is initialized to a non-NULL value when

  • The first disassembler option is given, or
  • Not initialized with disassembler options (in the first print_insn_riscv function call).

We can safely initialize default disassembler options prior to the print_insn_riscv function call and this per-instruction checking of riscv_gpr_names can be safely removed.

Instead, this patchset adds two new functions:

  • init_dis_state_for_arch
    (called when the architecture is changed)
  • init_dis_state_for_arch_and_options
    (called when either the architecture or an option is changed)

We can group common state initialization together with those.

1b. Minimize default architecture initialization (PATCH 2/7)

objdump reuses the disassembler returned by disassembler function.

That's good and benefits well from my optimizations.

However, by default, GDB (default_print_insn in gdb/arch-utils.c) assumes that disassembler function logic is simple and calls that function for every instruction to be disassembled. This is clearly a waste of time because it probes BFD (ELF information) and re-initializes riscv_rps_dis for every instruction.

To deal with this, we cache default architecture string and minimize calls to the riscv_parse_subset function (don't call it when arch is unchanged).

Long disas command on GDB benefits from this:

# Example: disassemble most of the code in the Linux kernel
file  /path/to/vmlinux
disas 0xffffffff80002000,0xffffffff80658aa4

On benchmark using GDB batch file like an example above, I measured big performance improvements (27-83% on various big RISC-V programs). Unfortunately, on interactive usecases of GDB, this improvement is rarely observable since we don't usually disassemble such a big chunk at once and the current disassembler is not very slow.

2. Move register name array initialization (PATCH 3/7)

This doesn't actually improve the performance but...

  • Register name initialization (involving Zfinx) is now clearer and
  • We can prepare future implementation of the RV32E disassembler.

3. Fix CSR hashtable initialization (PATCH 4/7)

Current disassembler intends to initialize the CSR hashtable when CSR name parsing is required at the first time. This is managed by a function-scope static variable init_csr but... there's a problem.

It's never set to true.

Because of this bug, current disassembler actually initializes the CSR hashtable every time when CSR name parsing is required.

After I fixed the bug, I got about 70% performance improvements when thousands of only CSR instructions are disassembled (CSR instructions are rare in general so real world performance improvement is not that high ― even hardly measurable).

When I fixed the bug, I found another issue (covered by the bug above).

When default_priv_spec is changed, CSR hashtable must be reinitialized so that it would reflect specified privileged specification. On objdump, there's no way to change it dynamically but GDB has one. Before I fix the bug above, CSR hashtable is generated every time so specified privileged specification is reflected immediately. However, now CSR hashtable has to be reinitialized manually.

So, I replaced init_csr with a file scope static variable is_init_csr and reset this when init_dis_state_for_arch_and_options is called.

Performance Improvements

On disassembling linked RISC-V ELF files using objdump, performance improvements achieved by this patchset is about 2-6%. Not bad for simple changes.

This is relative to the previous optimization.

objdump (near regular cases)

Program Improvements
Busybox 1.35.1 (RV64GC) 3.2-3.7%
OpenSBI 1.1 (generic fw_*.elf) 4.9-5.6%
Linux kernel 5.18 (vmlinux) 2.7-4.1%
Linux kernel 5.18 (vmlinux.o) (-1.2)-1.0%
glibc (libc.so.6) 3.1-3.5%
glibc (libc.a) (-1.0)-0.2%
newlib (libc.a) (-2.1)-(-0.5)%
Random files (/dev/urandom) 2.7-3.4%

objdump (special case)

Program Improvements
1M (1048576) CSR instructions 72.1%

gdb: near all code region of disas

Program Improvements
Linux kernel 5.18 (vmlinux) with debug info 27.6%
Linux kernel 5.18 (vmlinux) without debug info 83.4%
OpenSBI 1.1 (generic fw_*.elf) 57.4-61.4%
Clone this wiki locally