Hi Nelson, Thanks for the review comments. I didn't know the Changeling is generated automatically .... I'll remove them. Also will remove the redundant function call and use fprintf with stream. For the choice between -march=help vs. --help, I think Tsukasa suggested not to handle the -march=help in riscv_parse_subset(), since this function is also called by other functions. Like parsing the ".attribute arch" in .s files. https://patchwork.plctlab.org/project/binutils-gdb/patch/20240130063630.2931301-1-hau.hsu@sifive.com/#248218 We prefer -march=help because --help already prints too many informations. And as Kito said GCC has landed a similar option: https://github.com/gcc-mirror/gcc/commit/7af0f1e107a480fbfe882cb985603960114aefb5 Best, Hau Hsu > Nelson Chu 於 2024年3月7日 上午10:38 寫道： > > I don't know if we really need this or not, since you can give an extension without a version from -march, and then can get the default supported version in the output file. But if this really helps, I remember Tsukasa suggested doing this in --help, rather than add -march=help to show the information. He is exactly right, you can refer to what x86 did in gas/config/tc-i3866.c:show_arch. > > Try as --help, then you will get, > ... > -march=CPU[,+EXTENSION...] > generate code for CPU and EXTENSION, CPU is one of: > generic32, generic64, i386, i486, i586, i686, > ... > EXTENSION is combination of: > 8087, 287, 387, 687, cmov, fxsr, mmx, sse, sse2, > ... > > I assume we want, > ... > RISC-V options: > ... > -march=ISA set the RISC-V architecture, ISA is combination of: > e: 1.9 > i: 2.1, 2.0 > ... > zicbom: 1.0 > ... > -misa-spec=ISAspec set the RISC-V ISA spec (2.2, 20190608, 20191213) > -mpriv-spec=PRIVspec set the RISC-V privilege spec (1.9.1, 1.10, 1.11, 1.12) > ... > > If you don't want to show multiple extensions in the same line, then you probably don't need to care if the one line space is enough or not in the short-term, but eventually we will need to care someday. That is, probably just using fprintf for each extension should be okay in the short-term. > > Thanks > Nelson > > > On Wed, Mar 6, 2024 at 2:32 PM Hau Hsu > wrote: >> Ping~ >> >> > Kito Cheng > 於 2024年2月23日 下午2:48 寫道： >> > >> > LGTM :) >> > >> > On Fri, Feb 23, 2024 at 2:18 PM Hau Hsu > wrote: >> >> >> >> Use -march=help for gas to print all supported extensions and versions. >> >> >> >> Here is part of the output of `as -march=help`: >> >> All available -march extensions for RISC-V: >> >> e 1.9 >> >> i 2.1, 2.0 >> >> m 2.0 >> >> a 2.1, 2.0 >> >> f 2.2, 2.0 >> >> d 2.2, 2.0 >> >> q 2.2, 2.0 >> >> c 2.0 >> >> v 1.0 >> >> h 1.0 >> >> zicbom 1.0 >> >> zicbop 1.0 >> >> ... >> >> >> >> This patch assumes that the supported extensions with the same versions >> >> are listed together. For example: >> >> static struct riscv_supported_ext riscv_supported_std_ext[] = >> >> { >> >> ... >> >> {"i", ISA_SPEC_CLASS_20191213, 2, 1, 0 }, >> >> {"i", ISA_SPEC_CLASS_20190608, 2, 1, 0 }, >> >> {"i", ISA_SPEC_CLASS_2P2, 2, 0, 0 }, >> >> ... >> >> }; >> >> >> >> For the "i" extension, 2.1.0 with different spec class are listed together. >> >> This patch records the previous printed extension and version. If the >> >> current extension and version are the same as the previous one, skip >> >> printing. > > Add ChangeLog in the commit message, rather than modify the ChangeLog file directly, since it will be auto-generated when released. > >> >> --- >> >> bfd/ChangeLog | 9 ++++++ >> >> bfd/elfxx-riscv.c | 64 +++++++++++++++++++++++++++++++++++++++++++ >> >> bfd/elfxx-riscv.h | 3 ++ >> >> gas/ChangeLog | 4 +++ >> >> gas/config/tc-riscv.c | 6 ++++ >> >> 5 files changed, 86 insertions(+) >> >> >> >> diff --git a/bfd/ChangeLog b/bfd/ChangeLog >> >> index 97d0c585a56..66572927998 100644 >> >> --- a/bfd/ChangeLog >> >> +++ b/bfd/ChangeLog >> >> @@ -1,3 +1,12 @@ >> >> +2024-02-21 Hau Hsu > >> >> + >> >> + * elfxx-riscv.h (riscv_print_extensions): New declaration. >> >> + * elfxx-riscv.c (riscv_print_extensions): New function. Print >> >> + available extensions and versions. >> >> + (riscv_same_extension_version): New function. >> >> + (riscv_same_extension_diff_version): New function. >> >> + (riscv_valid_ext): New function. >> >> + > > Likewise. > >> >> 2024-01-15 Nick Clifton > >> >> >> >> * 2.42 branch point. >> >> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c >> >> index 9a121b47121..108e26d5b30 100644 >> >> --- a/bfd/elfxx-riscv.c >> >> +++ b/bfd/elfxx-riscv.c >> >> @@ -2051,6 +2051,70 @@ riscv_set_default_arch (riscv_parse_subset_t *rps) >> >> } >> >> } >> >> >> >> +static >> >> +bool riscv_same_extension_version( >> >> + const struct riscv_supported_ext* ext1, >> >> + const struct riscv_supported_ext* ext2) >> >> +{ >> >> + return (strcmp(ext1->name, ext2->name) == 0 >> >> + && ext1->major_version == ext2->major_version >> >> + && ext1->minor_version == ext2->minor_version); >> >> +} >> >> + >> >> +static >> >> +bool riscv_same_extension_diff_version( >> >> + const struct riscv_supported_ext* ext1, >> >> + const struct riscv_supported_ext* ext2) >> >> +{ >> >> + return (strcmp(ext1->name, ext2->name) == 0 >> >> + && !(ext1->major_version == ext2->major_version >> >> + && ext1->minor_version == ext2->minor_version)); >> >> +} >> >> + >> >> +static >> >> +bool riscv_valid_ext(const struct riscv_supported_ext *ext) >> >> +{ >> >> + return (ext->isa_spec_class != ISA_SPEC_CLASS_NONE >> >> + && ext->major_version != RISCV_UNKNOWN_VERSION >> >> + && ext->minor_version != RISCV_UNKNOWN_VERSION); >> >> +} >> >> + > > Seems no need to extract three functions, but each only for one conditional. > >> >> +void riscv_print_extensions(void) >> >> +{ >> >> + /* Record the previous pritned extension. >> >> + Print the current one if they are not the same. */ >> >> + const struct riscv_supported_ext *cur = NULL, *prev = NULL; >> >> + >> >> + int i, j; >> >> + printf ("All available -march extensions for RISC-V:"); >> >> + for (i = 0; riscv_all_supported_ext[i] != NULL; i++) >> >> + { >> >> + const struct riscv_supported_ext *exts = riscv_all_supported_ext[i]; >> >> + prev = NULL; >> >> + for (j = 0; exts[j].name != NULL; j++) >> >> + { >> >> + cur = &exts[j]; >> >> + if (!riscv_valid_ext (cur)) >> >> + continue; >> >> + >> >> + if (prev && riscv_same_extension_version (prev, cur)) >> >> + continue; >> >> + >> >> + if (!prev || !riscv_same_extension_diff_version (prev, cur)) >> >> + { >> >> + printf("\n\t%-40s%d.%d", cur->name, cur->major_version, cur->minor_version); > > fprintf with stream, and take care of the indent of --help. > >> >> + prev = &exts[j]; >> >> + } >> >> + else >> >> + { >> >> + printf(", %d.%d", cur->major_version, cur->minor_version); >> >> + prev = &exts[j]; >> >> + } >> >> + } >> >> + } >> >> + printf ("\n"); > > Likewise. > >> >> +} >> >> + >> >> /* Function for parsing ISA string. >> >> >> >> Return Value: >> >> diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h >> >> index ae4cbee7bc3..f3710ec7fab 100644 >> >> --- a/bfd/elfxx-riscv.h >> >> +++ b/bfd/elfxx-riscv.h >> >> @@ -121,6 +121,9 @@ riscv_multi_subset_supports (riscv_parse_subset_t *, enum riscv_insn_class); >> >> extern const char * >> >> riscv_multi_subset_supports_ext (riscv_parse_subset_t *, enum riscv_insn_class); >> >> >> >> +extern void >> >> +riscv_print_extensions(void); >> >> + >> >> extern void >> >> bfd_elf32_riscv_set_data_segment_info (struct bfd_link_info *, int *); >> >> extern void >> >> diff --git a/gas/ChangeLog b/gas/ChangeLog >> >> index 88e61083c44..cebd9d94ae5 100644 >> >> --- a/gas/ChangeLog >> >> +++ b/gas/ChangeLog >> >> @@ -1,3 +1,7 @@ >> >> +2024-02-21 Hau Hsu > >> >> + * gas/config/tc-riscv.c (md_parse_option): Parse 'help' keyword in >> >> + march to print available extensions and versions. >> >> + > > Moved to commit message. > >> >> 2024-02-19 Will Hawkins > >> >> >> >> * config/tc-bpf.c (parse_expression): Change switch to if so that >> >> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c >> >> index a4161420128..e0910f59157 100644 >> >> --- a/gas/config/tc-riscv.c >> >> +++ b/gas/config/tc-riscv.c >> >> @@ -4034,6 +4034,12 @@ md_parse_option (int c, const char *arg) >> >> switch (c) >> >> { >> >> case OPTION_MARCH: >> >> + /* List all avaiable archs. */ >> >> + if (strcmp (arg, "help") == 0) >> >> + { >> >> + riscv_print_extensions(); > > Handle it through --help. > >> >> + exit (EXIT_SUCCESS); >> >> + } >> >> default_arch_with_ext = arg; >> >> break; >> >> >> >> -- >> >> 2.31.1 >> >>