[6/6] RISC-V: Allow adding enabled extension via target arch attributes

Message ID 20240709124757.1405749-7-christoph.muellner@vrull.eu
State Committed
Delegated to: Jeff Law
Headers
Series RISC-V: Rewrite target arch attribute handling |

Checks

Context Check Description
rivoscibot/toolchain-ci-rivos-lint success Lint passed
rivoscibot/toolchain-ci-rivos-apply-patch fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply

Commit Message

Christoph Müllner July 9, 2024, 12:47 p.m. UTC
  The set of enabled extensions can be extended via target arch function
attributes by listing each extension with a '+' prefix and a comma as
list separator.  E.g.:
  __attribute__((target("arch=+zba,+zbb"))) void foo();

The programmer intends to ensure that one or more extensions
are enabled when building the code.  This is independent of the arch
string that is passed at build time via the -march= option.

Therefore, it is reasonable to allow enabling extensions via target arch
attributes, which have already been enabled via the -march= string.

The subset list code already supports such duplication for implied
extensions.  This patch adds an interface so the subset list
parser can be switched into a mode where duplication is allowed.

This commit fixes the following regressed test cases:
* gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-39.c
* gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-42.c
* gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-43.c
* gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-44.c
* gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-45.c
* gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-46.c

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc (riscv_subset_list::add):
	Allow adding enabled extension if m_allow_adding_dup is set.
	* config/riscv/riscv-subset.h: Add m_allow_adding_dup and setter.
	* config/riscv/riscv-target-attr.cc (riscv_target_attr_parser::parse_arch):
	Allow adding enabled extensions.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/pr115554.c: Change expected fail to expected pass.
	* gcc.target/riscv/target-attr-16.c: New test.

Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
---
 gcc/common/config/riscv/riscv-common.cc       | 17 +++++++-----
 gcc/config/riscv/riscv-subset.h               |  5 ++++
 gcc/config/riscv/riscv-target-attr.cc         |  3 +++
 gcc/testsuite/gcc.target/riscv/pr115554.c     |  2 --
 .../gcc.target/riscv/target-attr-16.c         | 26 +++++++++++++++++++
 5 files changed, 45 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/target-attr-16.c
  

Comments

Kito Cheng July 9, 2024, 2:55 p.m. UTC | #1
LGTM, thanks for fixing this...and will take a detailed review on the
remaining patch in the next few days :)


On Tue, Jul 9, 2024 at 8:51 PM Christoph Müllner
<christoph.muellner@vrull.eu> wrote:
>
> The set of enabled extensions can be extended via target arch function
> attributes by listing each extension with a '+' prefix and a comma as
> list separator.  E.g.:
>   __attribute__((target("arch=+zba,+zbb"))) void foo();
>
> The programmer intends to ensure that one or more extensions
> are enabled when building the code.  This is independent of the arch
> string that is passed at build time via the -march= option.
>
> Therefore, it is reasonable to allow enabling extensions via target arch
> attributes, which have already been enabled via the -march= string.
>
> The subset list code already supports such duplication for implied
> extensions.  This patch adds an interface so the subset list
> parser can be switched into a mode where duplication is allowed.
>
> This commit fixes the following regressed test cases:
> * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-39.c
> * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-42.c
> * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-43.c
> * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-44.c
> * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-45.c
> * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-46.c
>
> gcc/ChangeLog:
>
>         * common/config/riscv/riscv-common.cc (riscv_subset_list::add):
>         Allow adding enabled extension if m_allow_adding_dup is set.
>         * config/riscv/riscv-subset.h: Add m_allow_adding_dup and setter.
>         * config/riscv/riscv-target-attr.cc (riscv_target_attr_parser::parse_arch):
>         Allow adding enabled extensions.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/pr115554.c: Change expected fail to expected pass.
>         * gcc.target/riscv/target-attr-16.c: New test.
>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> ---
>  gcc/common/config/riscv/riscv-common.cc       | 17 +++++++-----
>  gcc/config/riscv/riscv-subset.h               |  5 ++++
>  gcc/config/riscv/riscv-target-attr.cc         |  3 +++
>  gcc/testsuite/gcc.target/riscv/pr115554.c     |  2 --
>  .../gcc.target/riscv/target-attr-16.c         | 26 +++++++++++++++++++
>  5 files changed, 45 insertions(+), 8 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/target-attr-16.c
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
> index c215484c287b..be4a87abee6d 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -677,12 +677,17 @@ riscv_subset_list::add (const char *subset, int major_version,
>           ext->minor_version = minor_version;
>         }
>        else
> -       error_at (
> -         m_loc,
> -         "%<-march=%s%>: extension %qs appear more than one time",
> -         m_arch,
> -         subset);
> -
> +       {
> +         /* The extension is already in the list.  */
> +         if (!m_allow_adding_dup
> +             || ext->major_version != major_version
> +             || ext->minor_version != minor_version)
> +           error_at (
> +             m_loc,
> +             "%<-march=%s%>: extension %qs appear more than one time",
> +             m_arch,
> +             subset);
> +       }
>        return;
>      }
>    else if (strlen (subset) == 1 && !standard_extensions_p (subset))
> diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
> index 256d28657460..c2d213c1734f 100644
> --- a/gcc/config/riscv/riscv-subset.h
> +++ b/gcc/config/riscv/riscv-subset.h
> @@ -62,6 +62,9 @@ private:
>    /* X-len of m_arch. */
>    unsigned m_xlen;
>
> +  /* Allow adding the same extension more than once.  */
> +  bool m_allow_adding_dup;
> +
>    riscv_subset_list (const char *, location_t);
>
>    const char *parsing_subset_version (const char *, const char *, unsigned *,
> @@ -106,6 +109,8 @@ public:
>
>    void set_loc (location_t);
>
> +  void set_allow_adding_dup (bool v) { m_allow_adding_dup = v; }
> +
>    void finalize ();
>  };
>
> diff --git a/gcc/config/riscv/riscv-target-attr.cc b/gcc/config/riscv/riscv-target-attr.cc
> index 317806143949..57235c9c0a7e 100644
> --- a/gcc/config/riscv/riscv-target-attr.cc
> +++ b/gcc/config/riscv/riscv-target-attr.cc
> @@ -109,6 +109,8 @@ riscv_target_attr_parser::parse_arch (const char *str)
>                       ? riscv_subset_list::parse (local_arch_str, m_loc)
>                       : riscv_cmdline_subset_list ()->clone ();
>        m_subset_list->set_loc (m_loc);
> +      m_subset_list->set_allow_adding_dup (true);
> +
>        while (token)
>         {
>           if (token[0] != '+')
> @@ -134,6 +136,7 @@ riscv_target_attr_parser::parse_arch (const char *str)
>           token = strtok_r (NULL, ",", &str_to_check);
>         }
>
> +      m_subset_list->set_allow_adding_dup (false);
>        m_subset_list->finalize ();
>        return true;
>      }
> diff --git a/gcc/testsuite/gcc.target/riscv/pr115554.c b/gcc/testsuite/gcc.target/riscv/pr115554.c
> index e7dcde6276fa..16d5f63aac0b 100644
> --- a/gcc/testsuite/gcc.target/riscv/pr115554.c
> +++ b/gcc/testsuite/gcc.target/riscv/pr115554.c
> @@ -9,5 +9,3 @@ extern
>  __attribute__((target("arch=+zbb")))
>  __attribute__((target("arch=+zbb")))
>  void bar(void);
> -
> -/* { dg-error "extension 'zbb' appear more than one time" "" { target *-*-* } 0 } */
> diff --git a/gcc/testsuite/gcc.target/riscv/target-attr-16.c b/gcc/testsuite/gcc.target/riscv/target-attr-16.c
> new file mode 100644
> index 000000000000..0904488a9ff1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/target-attr-16.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_zba" } */
> +
> +__attribute__((target("arch=+zba,+zbb")))
> +void foo1 (void)
> +{
> +}
> +
> +__attribute__((target("arch=+zbb,+zbb")))
> +void foo2 (void)
> +{
> +}
> +
> +__attribute__((target("arch=+zba")))
> +__attribute__((target("arch=+zbb")))
> +void foo (void)
> +{
> +}
> +
> +__attribute__((target("arch=+zbb")))
> +__attribute__((target("arch=+zbb")))
> +void bar (void)
> +{
> +}
> +
> +/* { dg-final { scan-assembler-times ".option arch, rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zaamo1p0_zalrsc1p0_zba1p0_zbb1p0" 4 } } */
> --
> 2.45.2
>
  

Patch

diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index c215484c287b..be4a87abee6d 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -677,12 +677,17 @@  riscv_subset_list::add (const char *subset, int major_version,
 	  ext->minor_version = minor_version;
 	}
       else
-	error_at (
-	  m_loc,
-	  "%<-march=%s%>: extension %qs appear more than one time",
-	  m_arch,
-	  subset);
-
+	{
+	  /* The extension is already in the list.  */
+	  if (!m_allow_adding_dup
+	      || ext->major_version != major_version
+	      || ext->minor_version != minor_version)
+	    error_at (
+	      m_loc,
+	      "%<-march=%s%>: extension %qs appear more than one time",
+	      m_arch,
+	      subset);
+	}
       return;
     }
   else if (strlen (subset) == 1 && !standard_extensions_p (subset))
diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
index 256d28657460..c2d213c1734f 100644
--- a/gcc/config/riscv/riscv-subset.h
+++ b/gcc/config/riscv/riscv-subset.h
@@ -62,6 +62,9 @@  private:
   /* X-len of m_arch. */
   unsigned m_xlen;
 
+  /* Allow adding the same extension more than once.  */
+  bool m_allow_adding_dup;
+
   riscv_subset_list (const char *, location_t);
 
   const char *parsing_subset_version (const char *, const char *, unsigned *,
@@ -106,6 +109,8 @@  public:
 
   void set_loc (location_t);
 
+  void set_allow_adding_dup (bool v) { m_allow_adding_dup = v; }
+
   void finalize ();
 };
 
diff --git a/gcc/config/riscv/riscv-target-attr.cc b/gcc/config/riscv/riscv-target-attr.cc
index 317806143949..57235c9c0a7e 100644
--- a/gcc/config/riscv/riscv-target-attr.cc
+++ b/gcc/config/riscv/riscv-target-attr.cc
@@ -109,6 +109,8 @@  riscv_target_attr_parser::parse_arch (const char *str)
 		      ? riscv_subset_list::parse (local_arch_str, m_loc)
 		      : riscv_cmdline_subset_list ()->clone ();
       m_subset_list->set_loc (m_loc);
+      m_subset_list->set_allow_adding_dup (true);
+
       while (token)
 	{
 	  if (token[0] != '+')
@@ -134,6 +136,7 @@  riscv_target_attr_parser::parse_arch (const char *str)
 	  token = strtok_r (NULL, ",", &str_to_check);
 	}
 
+      m_subset_list->set_allow_adding_dup (false);
       m_subset_list->finalize ();
       return true;
     }
diff --git a/gcc/testsuite/gcc.target/riscv/pr115554.c b/gcc/testsuite/gcc.target/riscv/pr115554.c
index e7dcde6276fa..16d5f63aac0b 100644
--- a/gcc/testsuite/gcc.target/riscv/pr115554.c
+++ b/gcc/testsuite/gcc.target/riscv/pr115554.c
@@ -9,5 +9,3 @@  extern
 __attribute__((target("arch=+zbb")))
 __attribute__((target("arch=+zbb")))
 void bar(void);
-
-/* { dg-error "extension 'zbb' appear more than one time" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/riscv/target-attr-16.c b/gcc/testsuite/gcc.target/riscv/target-attr-16.c
new file mode 100644
index 000000000000..0904488a9ff1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/target-attr-16.c
@@ -0,0 +1,26 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_zba" } */
+
+__attribute__((target("arch=+zba,+zbb")))
+void foo1 (void)
+{
+}
+
+__attribute__((target("arch=+zbb,+zbb")))
+void foo2 (void)
+{
+}
+
+__attribute__((target("arch=+zba")))
+__attribute__((target("arch=+zbb")))
+void foo (void)
+{
+}
+
+__attribute__((target("arch=+zbb")))
+__attribute__((target("arch=+zbb")))
+void bar (void)
+{
+}
+
+/* { dg-final { scan-assembler-times ".option arch, rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zaamo1p0_zalrsc1p0_zba1p0_zbb1p0" 4 } } */