| Message ID | 20240709124757.1405749-3-christoph.muellner@vrull.eu |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Rewrite target arch attribute handling | expand |
LGTM, thanks for simplifying this :) On Tue, Jul 9, 2024 at 8:48 PM Christoph Müllner <christoph.muellner@vrull.eu> wrote: > > We have a code duplication in riscv_set_arch_by_subset_list() and > riscv_parse_arch_string(), where the latter function parses an ISA string > into a subset_list before doing the same as the former function. > > riscv_parse_arch_string() is used to process command line options and > riscv_set_arch_by_subset_list() processes target attributes. > So, it is obvious that both functions should do the same. > Let's deduplicate the code to enforce this. > > gcc/ChangeLog: > > * common/config/riscv/riscv-common.cc (riscv_set_arch_by_subset_list): > Fix overlong line. > (riscv_parse_arch_string): Replace duplicated code by a call to > riscv_set_arch_by_subset_list. > > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> > --- > gcc/common/config/riscv/riscv-common.cc | 32 +++++-------------------- > 1 file changed, 6 insertions(+), 26 deletions(-) > > diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc > index 16bdb3fd2259..dfe960e51293 100644 > --- a/gcc/common/config/riscv/riscv-common.cc > +++ b/gcc/common/config/riscv/riscv-common.cc > @@ -1818,7 +1818,8 @@ riscv_set_arch_by_subset_list (riscv_subset_list *subset_list, > else if (subset_list->xlen () == 64) > opts->x_target_flags |= MASK_64BIT; > > - for (arch_ext_flag_tab = &riscv_ext_flag_table[0]; arch_ext_flag_tab->ext; > + for (arch_ext_flag_tab = &riscv_ext_flag_table[0]; > + arch_ext_flag_tab->ext; > ++arch_ext_flag_tab) > { > if (subset_list->lookup (arch_ext_flag_tab->ext)) > @@ -1842,30 +1843,6 @@ riscv_parse_arch_string (const char *isa, > if (!subset_list) > return; > > - if (opts) > - { > - const riscv_ext_flag_table_t *arch_ext_flag_tab; > - /* Clean up target flags before we set. */ > - for (arch_ext_flag_tab = &riscv_ext_flag_table[0]; > - arch_ext_flag_tab->ext; > - ++arch_ext_flag_tab) > - opts->*arch_ext_flag_tab->var_ref &= ~arch_ext_flag_tab->mask; > - > - if (subset_list->xlen () == 32) > - opts->x_target_flags &= ~MASK_64BIT; > - else if (subset_list->xlen () == 64) > - opts->x_target_flags |= MASK_64BIT; > - > - > - for (arch_ext_flag_tab = &riscv_ext_flag_table[0]; > - arch_ext_flag_tab->ext; > - ++arch_ext_flag_tab) > - { > - if (subset_list->lookup (arch_ext_flag_tab->ext)) > - opts->*arch_ext_flag_tab->var_ref |= arch_ext_flag_tab->mask; > - } > - } > - > /* Avoid double delete if current_subset_list equals cmdline_subset_list. */ > if (current_subset_list && current_subset_list != cmdline_subset_list) > delete current_subset_list; > @@ -1873,7 +1850,10 @@ riscv_parse_arch_string (const char *isa, > if (cmdline_subset_list) > delete cmdline_subset_list; > > - current_subset_list = cmdline_subset_list = subset_list; > + cmdline_subset_list = subset_list; > + /* current_subset_list is set in the call below. */ > + > + riscv_set_arch_by_subset_list (subset_list, opts); > } > > /* Return the riscv_cpu_info entry for CPU, NULL if not found. */ > -- > 2.45.2 >
diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc index 16bdb3fd2259..dfe960e51293 100644 --- a/gcc/common/config/riscv/riscv-common.cc +++ b/gcc/common/config/riscv/riscv-common.cc @@ -1818,7 +1818,8 @@ riscv_set_arch_by_subset_list (riscv_subset_list *subset_list, else if (subset_list->xlen () == 64) opts->x_target_flags |= MASK_64BIT; - for (arch_ext_flag_tab = &riscv_ext_flag_table[0]; arch_ext_flag_tab->ext; + for (arch_ext_flag_tab = &riscv_ext_flag_table[0]; + arch_ext_flag_tab->ext; ++arch_ext_flag_tab) { if (subset_list->lookup (arch_ext_flag_tab->ext)) @@ -1842,30 +1843,6 @@ riscv_parse_arch_string (const char *isa, if (!subset_list) return; - if (opts) - { - const riscv_ext_flag_table_t *arch_ext_flag_tab; - /* Clean up target flags before we set. */ - for (arch_ext_flag_tab = &riscv_ext_flag_table[0]; - arch_ext_flag_tab->ext; - ++arch_ext_flag_tab) - opts->*arch_ext_flag_tab->var_ref &= ~arch_ext_flag_tab->mask; - - if (subset_list->xlen () == 32) - opts->x_target_flags &= ~MASK_64BIT; - else if (subset_list->xlen () == 64) - opts->x_target_flags |= MASK_64BIT; - - - for (arch_ext_flag_tab = &riscv_ext_flag_table[0]; - arch_ext_flag_tab->ext; - ++arch_ext_flag_tab) - { - if (subset_list->lookup (arch_ext_flag_tab->ext)) - opts->*arch_ext_flag_tab->var_ref |= arch_ext_flag_tab->mask; - } - } - /* Avoid double delete if current_subset_list equals cmdline_subset_list. */ if (current_subset_list && current_subset_list != cmdline_subset_list) delete current_subset_list; @@ -1873,7 +1850,10 @@ riscv_parse_arch_string (const char *isa, if (cmdline_subset_list) delete cmdline_subset_list; - current_subset_list = cmdline_subset_list = subset_list; + cmdline_subset_list = subset_list; + /* current_subset_list is set in the call below. */ + + riscv_set_arch_by_subset_list (subset_list, opts); } /* Return the riscv_cpu_info entry for CPU, NULL if not found. */
We have a code duplication in riscv_set_arch_by_subset_list() and riscv_parse_arch_string(), where the latter function parses an ISA string into a subset_list before doing the same as the former function. riscv_parse_arch_string() is used to process command line options and riscv_set_arch_by_subset_list() processes target attributes. So, it is obvious that both functions should do the same. Let's deduplicate the code to enforce this. gcc/ChangeLog: * common/config/riscv/riscv-common.cc (riscv_set_arch_by_subset_list): Fix overlong line. (riscv_parse_arch_string): Replace duplicated code by a call to riscv_set_arch_by_subset_list. Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> --- gcc/common/config/riscv/riscv-common.cc | 32 +++++-------------------- 1 file changed, 6 insertions(+), 26 deletions(-)