| Message ID | 20200331090652.65252-2-kito.cheng@sifive.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] RISC-V: Update march parser | expand |
On Tue, Mar 31, 2020 at 2:07 AM Kito Cheng <kito.cheng@sifive.com> wrote: > - Implied rule are introduced into latest RISC-V isa spec. > > - Only implemented D implied F-extension. Zicsr and Zifence are not > implement yet, so the rule not included in this patch. When I try this patch, I see an error: rohan:2132$ ./xgcc -B./ -O -march=rv64imafdc -mabi=lp64d tmp.c /tmp/ccULN36f.s: Assembler messages: /tmp/ccULN36f.s:3: Fatal error: -march=rv64i2p0_m2p0_a2p0_f2p0_f2p0_d2p0_c2p0: ISA string is not in canonical order. `f' rohan:2133$ Looks like you need to make sure that we don't add the implied F if it is already there. Otherwise, this looks OK to me. We don't have support for these implied extensions in binutils yet. If we are adding it to gcc, we should probably add it to binutils too. Maybe you can ask Nelson to work on that. > + /* TODO: Implied extension might use differet version. */ differet -> different Jim
Hi Jim: > When I try this patch, I see an error: > > rohan:2132$ ./xgcc -B./ -O -march=rv64imafdc -mabi=lp64d tmp.c > /tmp/ccULN36f.s: Assembler messages: > /tmp/ccULN36f.s:3: Fatal error: > -march=rv64i2p0_m2p0_a2p0_f2p0_f2p0_d2p0_c2p0: ISA string is not in > canonical order. `f' > rohan:2133$ > > Looks like you need to make sure that we don't add the implied F if it > is already there. Otherwise, this looks OK to me. Thanks, I should add a testcase for that :) > > We don't have support for these implied extensions in binutils yet. > If we are adding it to gcc, we should probably add it to binutils too. > Maybe you can ask Nelson to work on that. I also found another issue: assembler will complain when passing -march=rv64id, because it does not accept implied extensions yet... $ riscv64-unknown-elf-gcc ~/hello.c -march=rv64id -c # GCC also invoke as with -march=rv64id Assembler messages: Fatal error: -march=rv64id: `d' extension requires `f' extension However if we require assembler support that means we need to bump the binutils version requirement again and might break backward compatibility, so I guess we can transform -march= via spec functions? I'll send a v2 patch with this approach. diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h index 567c23380fe..38644436f7c 100644 --- a/gcc/config/riscv/riscv.h +++ b/gcc/config/riscv/riscv.h @@ -46,7 +46,7 @@ along with GCC; see the file COPYING3. If not see --with-tune is ignored if -mtune is specified. */ #define OPTION_DEFAULT_SPECS \ {"tune", "%{!mtune=*:-mtune=%(VALUE)}" }, \ - {"arch", "%{!march=*:-march=%(VALUE)}" }, \ + {"arch", "%{!march=*:%:riscv_expand_arch(%(VALUE))}" }, \ {"abi", "%{!mabi=*:-mabi=%(VALUE)}" }, \ #ifdef IN_LIBGCC2 Thanks :) > > > + /* TODO: Implied extension might use differet version. */ > > differet -> different > > Jim
diff --git a/gcc/common/config/riscv/riscv-common.c b/gcc/common/config/riscv/riscv-common.c index a2e8d7003cc..24100cfae74 100644 --- a/gcc/common/config/riscv/riscv-common.c +++ b/gcc/common/config/riscv/riscv-common.c @@ -44,6 +44,20 @@ struct riscv_subset_t struct riscv_subset_t *next; }; +/* Type for implied ISA info. */ +struct riscv_implied_info_t +{ + const char *ext; + const char *implied_ext; +}; + +/* Implied ISA info, must end with NULL sentinel. */ +riscv_implied_info_t riscv_implied_info[] = +{ + {"d", "f"}, + {NULL, NULL} +}; + /* Subset list. */ class riscv_subset_list { @@ -73,6 +87,8 @@ private: const char *parse_multiletter_ext (const char *, const char *, const char *); + void handle_implied_ext (const char *, int, int); + public: ~riscv_subset_list (); @@ -394,11 +410,34 @@ riscv_subset_list::parse_std_ext (const char *p) subset[0] = std_ext; + handle_implied_ext (subset, major_version, minor_version); + add (subset, major_version, minor_version); } return p; } + +/* Check any implied extensions for EXT with version + MAJOR_VERSION.MINOR_VERSION. */ +void +riscv_subset_list::handle_implied_ext (const char *ext, + int major_version, + int minor_version) +{ + riscv_implied_info_t *implied_info; + for (implied_info = &riscv_implied_info[0]; + implied_info->ext; + ++implied_info) + { + if (strcmp (ext, implied_info->ext) != 0) + continue; + + /* TODO: Implied extension might use differet version. */ + add (implied_info->implied_ext, major_version, minor_version); + } +} + /* Parsing function for multi-letter extensions. Return Value: diff --git a/gcc/testsuite/gcc.target/riscv/arch-6.c b/gcc/testsuite/gcc.target/riscv/arch-6.c new file mode 100644 index 00000000000..b36dccbf46b --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/arch-6.c @@ -0,0 +1,5 @@ +/* { dg-do compile } */ +/* { dg-options "-O -march=rv32id -mabi=ilp32" } */ +int foo() +{ +} diff --git a/gcc/testsuite/gcc.target/riscv/attribute-11.c b/gcc/testsuite/gcc.target/riscv/attribute-11.c new file mode 100644 index 00000000000..a8649508b2f --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/attribute-11.c @@ -0,0 +1,6 @@ +/* { dg-do compile } */ +/* { dg-options "-O -mriscv-attribute -march=rv32id -mabi=ilp32" } */ +int foo() +{ +} +/* { dg-final { scan-assembler ".attribute arch, \"rv32i2p0_f2p0_d2p0\"" } } */
- Implied rule are introduced into latest RISC-V isa spec. - Only implemented D implied F-extension. Zicsr and Zifence are not implement yet, so the rule not included in this patch. gcc/ChangeLog * common/config/riscv/riscv-common.c (riscv_implied_info_t): New. (riscv_implied_info): New. (riscv_subset_list): Add handle_implied_ext. (riscv_subset_list::handle_implied_ext): New. (riscv_subset_list::parse_std_ext): Call handle_implied_ext. gcc/testsuite/ChangeLog * gcc.target/riscv/arch-6.c: New. * gcc.target/riscv/attribute-11.c: New. --- gcc/common/config/riscv/riscv-common.c | 39 +++++++++++++++++++ gcc/testsuite/gcc.target/riscv/arch-6.c | 5 +++ gcc/testsuite/gcc.target/riscv/attribute-11.c | 6 +++ 3 files changed, 50 insertions(+) create mode 100644 gcc/testsuite/gcc.target/riscv/arch-6.c create mode 100644 gcc/testsuite/gcc.target/riscv/attribute-11.c