[PATCH] RISC-V: Cache the latest mapping symbol and its boundary.

[PATCH] RISC-V: Cache the latest mapping symbol and its boundary.

[Kito Cheng]

This issue was reported from https://github.com/riscv-collab/riscv-gnu-toolchain/issues/1188

Current flow:
1) Scan any mapping symbol less than this instruciton.
2) If not found, did a backward search.

The flow seems not big issue, let run an example here:

$x:
0x0 a   <--- Found at step 1
0x4 b   <--- Not found in step 1, but found at step 2
0x8 c   <--- Not found in step 1, but found at step 2
$d
0x12 .word 1234 <-- Found at step 1

The instruciton didn't have the same address with mapping symbol will
still did backward search again and again.

So the new flow is:
1) Use the last mapping symbol status if the address is still within the range
   of the current mapping symbol.
2) Scan any mapping symbol less than this instruciton.
3) If not found, did a backward search.
4) If a proper mapping symbol is found in either step 2 or 3, find its boundary,
   and cache that.

Use the same example to run the new flow again:

$x:
0x0 a   <--- Found at step 2, the boundary is 0x12
0x4 b   <--- Cache hit at step 1, within the boundary.
0x8 c   <--- Cache hit at step 1, within the boundary.
$d
0x12 .word 1234 <-- Found at step 2, the boundary is the end of section.

The disassemble time of the test cases has been reduced from ~20 minutes to ~4
seconds.

opcode/ChangeLog
	PR 30282
	* riscv-dis.c (last_map_symbol_boundary): New.
	(last_map_state): New.
	(last_map_section): New.
	(riscv_search_mapping_symbol): Cache the result of latest
	mapping symbol.
---
 opcodes/riscv-dis.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 3aaa45f419c..f25993d1e45 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -64,6 +64,9 @@ struct riscv_private_data
 /* Used for mapping symbols.  */
 static int last_map_symbol = -1;
 static bfd_vma last_stop_offset = 0;
+static bfd_vma last_map_symbol_boundary = 0;
+static enum riscv_seg_mstate last_map_state = MAP_NONE;
+static asection *last_map_section = NULL;
 
 /* Register names as used by the disassembler.  */
 static const char * const *riscv_gpr_names;
@@ -868,6 +871,14 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
   int symbol = -1;
   int n;
 
+  /* Return the last map state if the address is still within the range of the
+     last mapping symbol.  */
+  if (last_map_section == info->section
+      && (memaddr < last_map_symbol_boundary))
+    return last_map_state;
+
+  last_map_section = info->section;
+
   /* Decide whether to print the data or instruction by default, in case
      we can not find the corresponding mapping symbols.  */
   mstate = MAP_DATA;
@@ -939,6 +950,36 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
 	}
     }
 
+  if (found)
+    {
+      /* Find the next mapping symbol to determine the boundary of this mapping
+	 symbol.  */
+
+      bool found_next = false;
+      /* Try to found next mapping symbol.  */
+      for (n = symbol + 1; n < info->symtab_size; n++)
+	{
+	  if (info->symtab[symbol]->section != info->symtab[n]->section)
+	    continue;
+
+	  bfd_vma addr = bfd_asymbol_value (info->symtab[n]);
+	  const char *sym_name = bfd_asymbol_name(info->symtab[n]);
+	  if (sym_name[0] == '$' && (sym_name[1] == 'x' || sym_name[1] == 'd'))
+	    {
+	      /* The next mapping symbol has been found, and it represents the
+		 boundary of this mapping symbol.  */
+	      found_next = true;
+	      last_map_symbol_boundary = addr;
+	      break;
+	    }
+	}
+
+      /* No further mapping symbol has been found, indicating that the boundary
+	 of the current mapping symbol is the end of this section.  */
+      if (!found_next)
+	last_map_symbol_boundary = info->section->vma + info->section->size;
+    }
+
   /* Save the information for next use.  */
   last_map_symbol = symbol;
   last_stop_offset = info->stop_offset;
@@ -1059,6 +1100,8 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
     set_default_riscv_dis_options ();
 
   mstate = riscv_search_mapping_symbol (memaddr, info);
+  /* Save the last mapping state.  */
+  last_map_state = mstate;
 
   /* Set the size to dump.  */
   if (mstate == MAP_DATA
-- 
2.39.2

Re: [PATCH] RISC-V: Cache the latest mapping symbol and its boundary.

[Nelson Chu]

[-- Attachment #1: Type: text/plain, Size: 4889 bytes --]

Thanks, committed after passing gcc/binutils regression of
riscv-gnu-toolchain.

Nelson

On Mon, Apr 17, 2023 at 8:16 PM Kito Cheng <kito.cheng@sifive.com> wrote:

> This issue was reported from
> https://github.com/riscv-collab/riscv-gnu-toolchain/issues/1188
>
> Current flow:
> 1) Scan any mapping symbol less than this instruciton.
> 2) If not found, did a backward search.
>
> The flow seems not big issue, let run an example here:
>
> $x:
> 0x0 a   <--- Found at step 1
> 0x4 b   <--- Not found in step 1, but found at step 2
> 0x8 c   <--- Not found in step 1, but found at step 2
> $d
> 0x12 .word 1234 <-- Found at step 1
>
> The instruciton didn't have the same address with mapping symbol will
> still did backward search again and again.
>
> So the new flow is:
> 1) Use the last mapping symbol status if the address is still within the
> range
>    of the current mapping symbol.
> 2) Scan any mapping symbol less than this instruciton.
> 3) If not found, did a backward search.
> 4) If a proper mapping symbol is found in either step 2 or 3, find its
> boundary,
>    and cache that.
>
> Use the same example to run the new flow again:
>
> $x:
> 0x0 a   <--- Found at step 2, the boundary is 0x12
> 0x4 b   <--- Cache hit at step 1, within the boundary.
> 0x8 c   <--- Cache hit at step 1, within the boundary.
> $d
> 0x12 .word 1234 <-- Found at step 2, the boundary is the end of section.
>
> The disassemble time of the test cases has been reduced from ~20 minutes
> to ~4
> seconds.
>
> opcode/ChangeLog
>         PR 30282
>         * riscv-dis.c (last_map_symbol_boundary): New.
>         (last_map_state): New.
>         (last_map_section): New.
>         (riscv_search_mapping_symbol): Cache the result of latest
>         mapping symbol.
> ---
>  opcodes/riscv-dis.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 3aaa45f419c..f25993d1e45 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -64,6 +64,9 @@ struct riscv_private_data
>  /* Used for mapping symbols.  */
>  static int last_map_symbol = -1;
>  static bfd_vma last_stop_offset = 0;
> +static bfd_vma last_map_symbol_boundary = 0;
> +static enum riscv_seg_mstate last_map_state = MAP_NONE;
> +static asection *last_map_section = NULL;
>
>  /* Register names as used by the disassembler.  */
>  static const char * const *riscv_gpr_names;
> @@ -868,6 +871,14 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
>    int symbol = -1;
>    int n;
>
> +  /* Return the last map state if the address is still within the range
> of the
> +     last mapping symbol.  */
> +  if (last_map_section == info->section
> +      && (memaddr < last_map_symbol_boundary))
> +    return last_map_state;
> +
> +  last_map_section = info->section;
> +
>    /* Decide whether to print the data or instruction by default, in case
>       we can not find the corresponding mapping symbols.  */
>    mstate = MAP_DATA;
> @@ -939,6 +950,36 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
>         }
>      }
>
> +  if (found)
> +    {
> +      /* Find the next mapping symbol to determine the boundary of this
> mapping
> +        symbol.  */
> +
> +      bool found_next = false;
> +      /* Try to found next mapping symbol.  */
> +      for (n = symbol + 1; n < info->symtab_size; n++)
> +       {
> +         if (info->symtab[symbol]->section != info->symtab[n]->section)
> +           continue;
> +
> +         bfd_vma addr = bfd_asymbol_value (info->symtab[n]);
> +         const char *sym_name = bfd_asymbol_name(info->symtab[n]);
> +         if (sym_name[0] == '$' && (sym_name[1] == 'x' || sym_name[1] ==
> 'd'))
> +           {
> +             /* The next mapping symbol has been found, and it represents
> the
> +                boundary of this mapping symbol.  */
> +             found_next = true;
> +             last_map_symbol_boundary = addr;
> +             break;
> +           }
> +       }
> +
> +      /* No further mapping symbol has been found, indicating that the
> boundary
> +        of the current mapping symbol is the end of this section.  */
> +      if (!found_next)
> +       last_map_symbol_boundary = info->section->vma +
> info->section->size;
> +    }
> +
>    /* Save the information for next use.  */
>    last_map_symbol = symbol;
>    last_stop_offset = info->stop_offset;
> @@ -1059,6 +1100,8 @@ print_insn_riscv (bfd_vma memaddr, struct
> disassemble_info *info)
>      set_default_riscv_dis_options ();
>
>    mstate = riscv_search_mapping_symbol (memaddr, info);
> +  /* Save the last mapping state.  */
> +  last_map_state = mstate;
>
>    /* Set the size to dump.  */
>    if (mstate == MAP_DATA
> --
> 2.39.2
>
>