Skip to content

Commit 4aabb92

Browse files
committed
RISC-V: Reorganize arch-related initialization and management
objdump reuses the disassembler function returned by the disassembler function. That's good and benefits well from various optimizations. However, by default, GDB (default_print_insn in gdb/arch-utils.c) assumes that the disassembler function logic is simple and calls that function for every instruction to be disassembled. This is clearly a waste of time because it probes BFD (ELF information) and re-initializes riscv_rps_dis for every instruction. After the support of mapping symbol with ISA string with commit 40f1a1a ("RISC-V: Output mapping symbols with ISA string."), this kind of per- instruction initialization of riscv_rps_dis can also occur on ELF files with mapping symbol + ISA string (in riscv_get_map_state function). It can be worse if the object is assembled using Binutils commit 0ce50fc ("RISC-V: Always generate mapping symbols at the start of the sections.") or later. To avoid repeated initialization, this commit - Caches the default / override architectures (in two "contexts") and - enables switching between them. riscv_dis_arch_context_t and new utility functions are defined for this purpose. We still have to read the ".riscv.attributes" section on every instruction on GDB but at least the most time-consuming part (updating the actual architecture from the architecture string) is avoided. Likewise, we still have to probe whether we have to update the architecture for every instruction with a mapping symbol with ISA string is suitable but at least we don't actually update the architecture unless necessary (either context itself or the ISA string of the current context is changed). This commit improves the disassembler performance well in those situations: - When long "disas" command is used on GDB - When ELF files with mapping symbols + ISA string is used This commit now implements new mapping symbol handling ("$x" means revert to the previous architecture [with "$x+arch"] if exists, otherwise revert to the default one usually read from an ELF attribute), the recent consensus made by Kito and Nelson. On the benchmark using GDB batch files, the author measured significant performance improvements (35-96% on various big RISC-V programs). Unfortunately, on interactive usecases of GDB, this improvement is rarely observable since we don't usually disassemble such a big chunk at once and the current disassembler is not very slow. On the benchmark using unstripped ELF files with mapping symbols + ISA string "$xrv...", performance improvements are significant and easily observable in the real world (150%-264% performance improvments). Aside from optimization, this commit, along with "RISC-V: Reorganize disassembler state initialization", makes state initialization clearer and makes further changes easier. Also, although not practical in the real world, this commit now allows multi-XLEN object disassembling if the object file has mapping symbols with ISA string and the machine is XLEN-neutral (e.g. objdump with "-m riscv" option). It may help testing Binutils / GAS. opcodes/ChangeLog: * riscv-dis.c (initial_default_arch): Special default architecture string which is handled separately. (riscv_dis_arch_context_t): New type to manage RISC-V architecture context for the disassembler. Two instance of this type is defined in this file - "default" and "override". (dis_arch_context_default): New. Architecture context inferred from either an ELF attribute or initial_default_arch. (dis_arch_context_override): New. Architecture context inferred from mapping symbols with ISA string. (dis_arch_context_current): New. A pointer to either dis_arch_context_default or dis_arch_context_override. (riscv_rps_dis): Add summary. Use initial values from the initial value of dis_arch_context_current - dis_arch_context_default. (from_last_map_symbol): Make it file scope to decide whether we should revert the architecture to the default in riscv_get_map_state function. (set_riscv_current_dis_arch_context): New function to update riscv_rps_dis and dis_arch_context_current. (set_riscv_dis_arch_context): New function to update the architecture for the given context. (update_riscv_dis_xlen): Consider dis_arch_context_current->xlen when guessing correct XLEN. (is_arch_changed): New. Set to true if the architecture is changed. (init_riscv_dis_state_for_arch): New function to track whether the architecture string is changed. (init_riscv_dis_state_for_arch_and_options): Keep track of the architecture string change and update XLEN if it has changed. (update_riscv_dis_arch): New function to set both the architecture and the context. Call initialization functions if needed. (riscv_get_map_state): Add update argument. Keep track of the mapping symbols with ISA string and update the architecture and the context if required. (riscv_search_mapping_symbol): Move from_last_map_symbol to file scope. Call riscv_get_map_state function with architecture and context updates enabled. (riscv_data_length): Call riscv_get_map_state function with architecture and context updates disabled. (riscv_get_disassembler): Add an error handling on Tag_RISCV_arch. Call update_riscv_dis_arch function to update the architecture and the context. Co-developed-by: Nelson Chu <[email protected]>
1 parent f468e01 commit 4aabb92

File tree

1 file changed

+152
-22
lines changed

1 file changed

+152
-22
lines changed

opcodes/riscv-dis.c

Lines changed: 152 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232
#include <stdint.h>
3333
#include <ctype.h>
3434

35+
/* Default architecture string (if not available). */
36+
static const char *const initial_default_arch = "rv64gc";
37+
3538
/* Current XLEN for the disassembler. */
3639
static unsigned xlen = 0;
3740

@@ -48,14 +51,36 @@ static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_DRAFT - 1;
4851
(as specified by the ELF attributes or the `priv-spec' option). */
4952
static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
5053

51-
static riscv_subset_list_t riscv_subsets;
54+
/* RISC-V disassembler architecture context type. */
55+
typedef struct
56+
{
57+
const char *arch_str;
58+
const char *default_arch;
59+
riscv_subset_list_t subsets;
60+
unsigned xlen;
61+
bool no_xlen_if_default;
62+
} riscv_dis_arch_context_t;
63+
64+
/* Context: default (either initial_default_arch or ELF attribute). */
65+
static riscv_dis_arch_context_t dis_arch_context_default
66+
= { NULL, initial_default_arch, {}, 0, true };
67+
68+
/* Context: override (mapping symbols with ISA string). */
69+
static riscv_dis_arch_context_t dis_arch_context_override
70+
= { NULL, NULL, {}, 0, false };
71+
72+
/* Pointer to the current disassembler architecture context. */
73+
static riscv_dis_arch_context_t *dis_arch_context_current
74+
= &dis_arch_context_default;
75+
76+
/* RISC-V ISA string parser structure (current). */
5277
static riscv_parse_subset_t riscv_rps_dis =
5378
{
54-
&riscv_subsets, /* subset_list. */
55-
opcodes_error_handler,/* error_handler. */
56-
&xlen, /* xlen. */
57-
&default_isa_spec, /* isa_spec. */
58-
false, /* check_unknown_prefixed_ext. */
79+
&(dis_arch_context_default.subsets), /* subset_list. */
80+
opcodes_error_handler, /* error_handler. */
81+
&(dis_arch_context_default.xlen), /* xlen. */
82+
&default_isa_spec, /* isa_spec. */
83+
false, /* check_unknown_prefixed_ext. */
5984
};
6085

6186
/* Private data structure for the RISC-V disassembler. */
@@ -71,6 +96,7 @@ struct riscv_private_data
7196
/* Used for mapping symbols. */
7297
static int last_map_symbol = -1;
7398
static bfd_vma last_stop_offset = 0;
99+
static bool from_last_map_symbol = false;
74100

75101
/* Register names as used by the disassembler. */
76102
static const char * const *riscv_gpr_names;
@@ -83,6 +109,59 @@ static bool no_aliases = false;
83109
static bool is_numeric = false;
84110

85111

112+
/* Set current disassembler context (dis_arch_context_current).
113+
Return true if successfully updated. */
114+
115+
static bool
116+
set_riscv_current_dis_arch_context (riscv_dis_arch_context_t* context)
117+
{
118+
if (context == dis_arch_context_current)
119+
return false;
120+
dis_arch_context_current = context;
121+
riscv_rps_dis.subset_list = &(context->subsets);
122+
riscv_rps_dis.xlen = &(context->xlen);
123+
return true;
124+
}
125+
126+
/* Update riscv_dis_arch_context_t by an ISA string.
127+
Return true if the architecture is updated by arch. */
128+
129+
static bool
130+
set_riscv_dis_arch_context (riscv_dis_arch_context_t *context,
131+
const char *arch)
132+
{
133+
/* Check whether the architecture is changed and
134+
return false if the architecture will not be changed. */
135+
if (context->arch_str)
136+
{
137+
if (context->default_arch && arch == context->default_arch)
138+
return false;
139+
if (strcmp (context->arch_str, arch) == 0)
140+
return false;
141+
}
142+
/* Update architecture string. */
143+
if (context->arch_str != context->default_arch)
144+
free ((void *) context->arch_str);
145+
context->arch_str = (arch != context->default_arch)
146+
? xstrdup (arch)
147+
: context->default_arch;
148+
/* Update other contents (subset list and XLEN). */
149+
riscv_subset_list_t *prev_subsets = riscv_rps_dis.subset_list;
150+
unsigned *prev_xlen = riscv_rps_dis.xlen;
151+
riscv_rps_dis.subset_list = &(context->subsets);
152+
riscv_rps_dis.xlen = &(context->xlen);
153+
context->xlen = 0;
154+
riscv_release_subset_list (&context->subsets);
155+
riscv_parse_subset (&riscv_rps_dis, context->arch_str);
156+
riscv_rps_dis.subset_list = prev_subsets;
157+
riscv_rps_dis.xlen = prev_xlen;
158+
/* Special handling on the default architecture. */
159+
if (context->no_xlen_if_default && arch == context->default_arch)
160+
context->xlen = 0;
161+
return true;
162+
}
163+
164+
86165
/* Guess and update current XLEN. */
87166

88167
static void
@@ -95,9 +174,13 @@ update_riscv_dis_xlen (struct disassemble_info *info)
95174
This is only effective if XLEN-specific BFD machine architecture is
96175
chosen. If XLEN-neutral (like riscv), BFD machine architecture is
97176
ignored on XLEN selection.
98-
2. ELF class in dummy ELF header. */
177+
2. Non-default RISC-V architecture string set by either an ELF
178+
attribute or a mapping symbol with ISA string.
179+
3. ELF class in dummy ELF header. */
99180
if (xlen_by_mach != 0)
100181
xlen = xlen_by_mach;
182+
else if (dis_arch_context_current->xlen != 0)
183+
xlen = dis_arch_context_current->xlen;
101184
else if (xlen_by_elf != 0)
102185
xlen = xlen_by_elf;
103186
else if (info != NULL && info->section != NULL)
@@ -107,18 +190,50 @@ update_riscv_dis_xlen (struct disassemble_info *info)
107190
}
108191
}
109192

193+
/* Initialization (for arch). */
194+
195+
static bool is_arch_changed = false;
196+
197+
static void
198+
init_riscv_dis_state_for_arch (void)
199+
{
200+
is_arch_changed = true;
201+
}
202+
110203
/* Initialization (for arch and options). */
111204

112205
static void
113206
init_riscv_dis_state_for_arch_and_options (void)
114207
{
208+
/* If the architecture string is changed, update XLEN. */
209+
if (is_arch_changed)
210+
update_riscv_dis_xlen (NULL);
115211
/* Set GPR register names to disassemble. */
116212
riscv_gpr_names = is_numeric ? riscv_gpr_names_numeric : riscv_gpr_names_abi;
117213
/* Set FPR register names to disassemble. */
118214
riscv_fpr_names
119215
= !riscv_subset_supports (&riscv_rps_dis, "zfinx")
120216
? (is_numeric ? riscv_fpr_names_numeric : riscv_fpr_names_abi)
121217
: riscv_gpr_names;
218+
/* Save previous options and mark them "unchanged". */
219+
is_arch_changed = false;
220+
}
221+
222+
/* Update architecture for disassembler with its context.
223+
Call initialization functions if either:
224+
- the architecture for current context is changed or
225+
- context is updated to a new one. */
226+
227+
static void
228+
update_riscv_dis_arch (riscv_dis_arch_context_t *context, const char *arch)
229+
{
230+
if ((set_riscv_dis_arch_context (context, arch)
231+
&& dis_arch_context_current == context)
232+
|| set_riscv_current_dis_arch_context (context))
233+
{
234+
init_riscv_dis_state_for_arch ();
235+
init_riscv_dis_state_for_arch_and_options ();
236+
}
122237
}
123238

124239

@@ -872,7 +987,8 @@ riscv_get_map_state_by_name (const char *name, const char** arch)
872987
static bool
873988
riscv_get_map_state (int n,
874989
enum riscv_seg_mstate *state,
875-
struct disassemble_info *info)
990+
struct disassemble_info *info,
991+
bool update)
876992
{
877993
const char *name, *arch = NULL;
878994

@@ -886,12 +1002,25 @@ riscv_get_map_state (int n,
8861002
if (newstate == MAP_NONE)
8871003
return false;
8881004
*state = newstate;
889-
if (arch)
890-
{
891-
riscv_release_subset_list (&riscv_subsets);
892-
riscv_parse_subset (&riscv_rps_dis, arch);
893-
init_riscv_dis_state_for_arch_and_options ();
894-
}
1005+
if (newstate == MAP_INSN && update)
1006+
{
1007+
if (arch)
1008+
{
1009+
/* Override the architecture. */
1010+
update_riscv_dis_arch (&dis_arch_context_override, arch);
1011+
}
1012+
else if (!from_last_map_symbol
1013+
&& set_riscv_current_dis_arch_context (&dis_arch_context_default))
1014+
{
1015+
/* Revert to the default architecture and call init functions if:
1016+
- there's no ISA string in the mapping symbol,
1017+
- mapping symbol is not reused and
1018+
- current disassembler context is changed to the default one.
1019+
This is a shortcut path to avoid full update_riscv_dis_arch. */
1020+
init_riscv_dis_state_for_arch ();
1021+
init_riscv_dis_state_for_arch_and_options ();
1022+
}
1023+
}
8951024
return true;
8961025
}
8971026

@@ -903,7 +1032,6 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
9031032
struct disassemble_info *info)
9041033
{
9051034
enum riscv_seg_mstate mstate;
906-
bool from_last_map_symbol;
9071035
bool found = false;
9081036
int symbol = -1;
9091037
int n;
@@ -943,7 +1071,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
9431071
/* We have searched all possible symbols in the range. */
9441072
if (addr > memaddr)
9451073
break;
946-
if (riscv_get_map_state (n, &mstate, info))
1074+
if (riscv_get_map_state (n, &mstate, info, true))
9471075
{
9481076
symbol = n;
9491077
found = true;
@@ -970,7 +1098,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
9701098
if (addr < (info->section ? info->section->vma : 0))
9711099
break;
9721100
/* Stop searching once we find the closed mapping symbol. */
973-
if (riscv_get_map_state (n, &mstate, info))
1101+
if (riscv_get_map_state (n, &mstate, info, true))
9741102
{
9751103
symbol = n;
9761104
found = true;
@@ -1006,7 +1134,7 @@ riscv_data_length (bfd_vma memaddr,
10061134
{
10071135
bfd_vma addr = bfd_asymbol_value (info->symtab[n]);
10081136
if (addr > memaddr
1009-
&& riscv_get_map_state (n, &m, info))
1137+
&& riscv_get_map_state (n, &m, info, false))
10101138
{
10111139
if (addr - memaddr < length)
10121140
length = addr - memaddr;
@@ -1130,7 +1258,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
11301258
disassembler_ftype
11311259
riscv_get_disassembler (bfd *abfd)
11321260
{
1133-
const char *default_arch = "rv64gc";
1261+
const char *default_arch = initial_default_arch;
11341262

11351263
if (abfd && bfd_get_flavour (abfd) == bfd_target_elf_flavour)
11361264
{
@@ -1146,12 +1274,14 @@ riscv_get_disassembler (bfd *abfd)
11461274
attr[Tag_c].i,
11471275
&default_priv_spec);
11481276
default_arch = attr[Tag_RISCV_arch].s;
1277+
/* For ELF files with (somehow) no architecture string
1278+
in the attributes, use the default value. */
1279+
if (!default_arch)
1280+
default_arch = initial_default_arch;
11491281
}
11501282
}
11511283

1152-
riscv_release_subset_list (&riscv_subsets);
1153-
riscv_parse_subset (&riscv_rps_dis, default_arch);
1154-
init_riscv_dis_state_for_arch_and_options ();
1284+
update_riscv_dis_arch (&dis_arch_context_default, default_arch);
11551285
return print_insn_riscv;
11561286
}
11571287

0 commit comments

Comments
 (0)