[v2] RISC-V: Avoid updating state until symbol is found

Message ID 20231129224213.1345331-1-patrick@rivosinc.com
State Accepted
Headers
Series [v2] RISC-V: Avoid updating state until symbol is found |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Patrick O'Neill Nov. 29, 2023, 10:42 p.m. UTC
  Currently objdump gets and updates the map state once per symbol. Updating the
state (partiularly riscv_parse_subset) is expensive and grows quadratically
since we iterate over all symbols. By deferring this until once we've found the
symbol of interest, we can reduce the time to dump a 4k insn file of .norvc and
.rvc insns from ~47 seconds to ~0.13 seconds.

opcodes/ChangeLog:

	* riscv-dis.c (riscv_get_map_state): Remove state updating logic
	and rename to riscv_is_valid_mapping_symbol.
	(riscv_update_map_state): Add state updating logic to seperate function.
	(riscv_search_mapping_symbol): Use new riscv_update_map_state.
	(riscv_data_length): Ditto.

Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
---
Tested using rv64gcv make report-linux and make report-binutils-linux.
Binutils hash used/applied to: c618a1c548193d2a6a8c3d909a3d1c620a156b5d
GCC hash used: eecdd96c8d1de244e21212a830e51062b3e444c5

v2 changelog:
- Remove unneeded riscv_update_state calls on !found
- Rename riscv_get_map_state -> riscv_is_valid_mapping_symbol
---
 opcodes/riscv-dis.c | 45 +++++++++++++++++++++++++++++++++------------
 1 file changed, 33 insertions(+), 12 deletions(-)
  

Patch

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 6fa9855e3cd..88eaf97d4a4 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -873,20 +873,20 @@  riscv_disassemble_insn (bfd_vma memaddr,
   return insnlen;
 }
 
-/* Return true if we find the suitable mapping symbol,
-   and also update the STATE.  Otherwise, return false.  */
+/* If we find the suitable mapping symbol update the STATE.
+   Otherwise, do nothing.  */
 
-static bool
-riscv_get_map_state (int n,
-		     enum riscv_seg_mstate *state,
-		     struct disassemble_info *info)
+static void
+riscv_update_map_state (int n,
+			enum riscv_seg_mstate *state,
+			struct disassemble_info *info)
 {
   const char *name;
 
   /* If the symbol is in a different section, ignore it.  */
   if (info->section != NULL
       && info->section != info->symtab[n]->section)
-    return false;
+    return;
 
   name = bfd_asymbol_name(info->symtab[n]);
   if (strcmp (name, "$x") == 0)
@@ -913,10 +913,26 @@  riscv_get_map_state (int n,
       else
 	riscv_parse_subset (&riscv_rps_dis, name + 2);
     }
-  else
+}
+
+/* Return true if we find the suitable mapping symbol.
+   Otherwise, return false.  */
+
+static bool
+riscv_is_valid_mapping_symbol (int n,
+			       struct disassemble_info *info)
+{
+  const char *name;
+
+  /* If the symbol is in a different section, ignore it.  */
+  if (info->section != NULL
+      && info->section != info->symtab[n]->section)
     return false;
 
-  return true;
+  name = bfd_asymbol_name(info->symtab[n]);
+  return (strcmp (name, "$x") == 0
+	  || strcmp (name, "$d") == 0
+	  || strncmp (name, "$xrv", 4) == 0);
 }
 
 /* Check the sorted symbol table (sorted by the symbol value), find the
@@ -975,7 +991,7 @@  riscv_search_mapping_symbol (bfd_vma memaddr,
       /* We have searched all possible symbols in the range.  */
       if (addr > memaddr)
 	break;
-      if (riscv_get_map_state (n, &mstate, info))
+      if (riscv_is_valid_mapping_symbol (n, info))
 	{
 	  symbol = n;
 	  found = true;
@@ -985,6 +1001,9 @@  riscv_search_mapping_symbol (bfd_vma memaddr,
 	}
     }
 
+  if (found)
+    riscv_update_map_state (symbol, &mstate, info);
+
   /* We can not find the suitable mapping symbol above.  Therefore, we
      look forwards and try to find it again, but don't go past the start
      of the section.  Otherwise a data section without mapping symbols
@@ -1002,10 +1021,11 @@  riscv_search_mapping_symbol (bfd_vma memaddr,
 	  if (addr < (info->section ? info->section->vma : 0))
 	    break;
 	  /* Stop searching once we find the closed mapping symbol.  */
-	  if (riscv_get_map_state (n, &mstate, info))
+	  if (riscv_is_valid_mapping_symbol (n, info))
 	    {
 	      symbol = n;
 	      found = true;
+	      riscv_update_map_state (symbol, &mstate, info);
 	      break;
 	    }
 	}
@@ -1068,11 +1088,12 @@  riscv_data_length (bfd_vma memaddr,
 	{
 	  bfd_vma addr = bfd_asymbol_value (info->symtab[n]);
 	  if (addr > memaddr
-	      && riscv_get_map_state (n, &m, info))
+	      && riscv_is_valid_mapping_symbol (n, info))
 	    {
 	      if (addr - memaddr < length)
 		length = addr - memaddr;
 	      found = true;
+	      riscv_update_map_state (n, &m, info);
 	      break;
 	    }
 	}