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

[Nelson Chu <nelson@rivosinc.com>]

[-- 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
>
>