[2/2] RISC-V: Handle implied extension for -march parser.

Message ID 20200331090652.65252-2-kito.cheng@sifive.com
State New
Headers show
Series
  • [1/2] RISC-V: Update march parser
Related show

Commit Message

Kito Cheng March 31, 2020, 9:06 a.m.
- 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

-- 
2.25.2

Comments

Jim Wilson April 6, 2020, 10:51 p.m. | #1
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
Kito Cheng April 7, 2020, 2:10 p.m. | #2
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

Patch

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\"" } } */