* [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).