* [RFC PATCH 0/1] gdb/riscv: Cache per-BFD disassembler @ 2022-10-09 3:59 Tsukasa OI 2022-10-09 3:59 ` [RFC PATCH 1/1] " Tsukasa OI 0 siblings, 1 reply; 4+ messages in thread From: Tsukasa OI @ 2022-10-09 3:59 UTC (permalink / raw) To: Tsukasa OI, Andrew Burgess, Palmer Dabbelt; +Cc: gdb-patches Hello, On RISC-V, calling the disassembler function (libopcodes) is not a small cost. This is because riscv_get_disassembler function sets the default architecture from given BFD's .riscv.attributes section. However, by default, GDB calls this function for every instruction. This commit replaces RISC-V's disassembler function and stop calling riscv_get_disassembler function if current BFD has not changed. It expects around 30-80% improvements on disassembling relatively large chunk of RISC-V code but most of them will be obscured by a RISC-V disassembler optimization the author is currently working on. Still, 3-5% of performance improvements will remain (due to reduced BFD section reads). [REQUEST FOR COMMENTS] I'm confident that most of BFD contents won't change so dynamically on GDB and caching itself is not a bad idea. Still, I'm not sure whether this usage of "static" variables is okay. I would like to hear your thoughts. Thanks, Tsukasa Tsukasa OI (1): gdb/riscv: Cache per-BFD disassembler gdb/riscv-tdep.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) base-commit: c10a862f17847bc9c50d680c87b87dc51ae4b95e -- 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC PATCH 1/1] gdb/riscv: Cache per-BFD disassembler 2022-10-09 3:59 [RFC PATCH 0/1] gdb/riscv: Cache per-BFD disassembler Tsukasa OI @ 2022-10-09 3:59 ` Tsukasa OI 2022-10-14 18:39 ` Tom Tromey 0 siblings, 1 reply; 4+ messages in thread From: Tsukasa OI @ 2022-10-09 3:59 UTC (permalink / raw) To: Tsukasa OI, Andrew Burgess, Palmer Dabbelt; +Cc: gdb-patches On RISC-V, calling the disassembler function (libopcodes) is not a small cost. This is because riscv_get_disassembler function sets the default architecture from given BFD's .riscv.attributes section. However, by default, GDB calls this function for every instruction. This commit replaces RISC-V's disassembler function and stop calling riscv_get_disassembler function if current BFD has not changed. It expects around 30-80% improvements on disassembling relatively large chunk of RISC-V code but most of them will be obscured by a RISC-V disassembler optimization the author is currently working on. Still, 3-5% of performance improvements will remain (due to reduced BFD section reads). --- gdb/riscv-tdep.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index feca17d9141..ed68a52df4c 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -55,6 +55,7 @@ #include "cli/cli-decode.h" #include "observable.h" #include "prologue-value.h" +#include "progspace.h" #include "arch/riscv.h" #include "riscv-ravenscar-thread.h" @@ -1308,6 +1309,27 @@ riscv_print_one_register_info (struct gdbarch *gdbarch, gdb_printf (file, "\n"); } +/* Calling disassembler function on RISC-V is not fast as others. + We cache the disassembler function as long as current BFD is the same. */ + +static int +riscv_print_insn (bfd_vma memaddr, disassemble_info *info) +{ + static disassembler_ftype disassemble_fn = NULL; + static bfd *abfd = NULL; + bfd *ebfd = current_program_space->exec_bfd (); + + if (disassemble_fn == NULL || abfd != ebfd) + { + disassemble_fn = disassembler ( + info->arch, info->endian == BFD_ENDIAN_BIG, info->mach, ebfd); + abfd = ebfd; + } + + gdb_assert (disassemble_fn != NULL); + return (*disassemble_fn) (memaddr, info); +} + /* Return true if REGNUM is a valid CSR register. The CSR register space is sparsely populated, so not every number is a named CSR. */ @@ -3926,6 +3948,7 @@ riscv_gdbarch_init (struct gdbarch_info info, set_gdbarch_pc_regnum (gdbarch, RISCV_PC_REGNUM); set_gdbarch_print_registers_info (gdbarch, riscv_print_registers_info); + set_gdbarch_print_insn (gdbarch, riscv_print_insn); set_tdesc_pseudo_register_name (gdbarch, riscv_pseudo_register_name); set_tdesc_pseudo_register_type (gdbarch, riscv_pseudo_register_type); -- 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH 1/1] gdb/riscv: Cache per-BFD disassembler 2022-10-09 3:59 ` [RFC PATCH 1/1] " Tsukasa OI @ 2022-10-14 18:39 ` Tom Tromey 2022-10-16 15:31 ` Tsukasa OI 0 siblings, 1 reply; 4+ messages in thread From: Tom Tromey @ 2022-10-14 18:39 UTC (permalink / raw) To: Tsukasa OI via Gdb-patches >>>>> Tsukasa OI via Gdb-patches <gdb-patches@sourceware.org> writes: > +static int > +riscv_print_insn (bfd_vma memaddr, disassemble_info *info) > +{ > + static disassembler_ftype disassemble_fn = NULL; > + static bfd *abfd = NULL; This seems mildly dangerous, in that a BFD could be destroyed, then a new one created with the same address. It's better to cache things using the registry system. See gdb/registry.h. You can look for "registry<bfd>" for examples. Tom ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH 1/1] gdb/riscv: Cache per-BFD disassembler 2022-10-14 18:39 ` Tom Tromey @ 2022-10-16 15:31 ` Tsukasa OI 0 siblings, 0 replies; 4+ messages in thread From: Tsukasa OI @ 2022-10-16 15:31 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 2022/10/15 3:39, Tom Tromey wrote: >>>>>> Tsukasa OI via Gdb-patches <gdb-patches@sourceware.org> writes: > >> +static int >> +riscv_print_insn (bfd_vma memaddr, disassemble_info *info) >> +{ >> + static disassembler_ftype disassemble_fn = NULL; >> + static bfd *abfd = NULL; > > This seems mildly dangerous, in that a BFD could be destroyed, then a > new one created with the same address. > > It's better to cache things using the registry system. > See gdb/registry.h. You can look for "registry<bfd>" for examples. > > Tom > Thanks for letting me know. Because my opcodes-side optimization on RISC-V will hide most of the performance improvements made by that proposed patch, I will need to redo the benchmark after I write a patch to use registry system. If registry cost is too high, I will have to scrap this idea instead of using the registry system. Thanks, Tsukasa ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-16 15:31 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-09 3:59 [RFC PATCH 0/1] gdb/riscv: Cache per-BFD disassembler Tsukasa OI 2022-10-09 3:59 ` [RFC PATCH 1/1] " Tsukasa OI 2022-10-14 18:39 ` Tom Tromey 2022-10-16 15:31 ` Tsukasa OI
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).