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.muell...@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.muell...@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
>
