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 >