From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102b.google.com (mail-pj1-x102b.google.com [IPv6:2607:f8b0:4864:20::102b]) by sourceware.org (Postfix) with ESMTPS id E49923858C78 for ; Tue, 12 Dec 2023 14:00:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E49923858C78 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=vrull.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vrull.eu ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E49923858C78 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::102b ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702389623; cv=none; b=x3bS7uCFiCAkOo4wxnKpHwB0c361wRs2JdRH65w/P18oRKwpq9oOQFfINdjJGCaH+dkTT4t0WYHuf2124hje8Y2eysahgxrZYo7FsoCp0Fp7iQr1P5CXKrPhUm8n6Dbu4/Sr3j15NAsOkWCpy2hv7jElXGUMjYISQpm5FZPqYUI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702389623; c=relaxed/simple; bh=cUlshUhI7Aas3PEgtAUiqUXfQRBx1iCVPcxeAQy7MN0=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=osVUzdWMIdCmsHpxGpSq/ULpwuNHAZK3kJzAf+ROrVXC3wgK9SCJJH4sfbLGFNXXZFQ16HLILBJa/AuXzJ3oGa37PKlSmtPMg9Qb6RgaenBChnCQOxSRsDkGvZ/Zshg5CiOQes2ucXMRLGlL6MdbJKkS4vJwXhI2zAe0uJXgi1A= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pj1-x102b.google.com with SMTP id 98e67ed59e1d1-286f22c52c7so3980205a91.2 for ; Tue, 12 Dec 2023 06:00:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vrull.eu; s=google; t=1702389619; x=1702994419; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=u3GNzdEDqQxiCOIUcXvEz+JlsRFHHXMx8I/HEqc6Pqw=; b=cdt3b8J99ewEV+pEQ5G3URCjEUyfx+4oi2Jht0+zKcv4NAbqycwHdn0rLSD8Mlsllx Qb7WicJ+1HxxyUmiJH1+IiY8/KbgaMq0WiF6cgwJoNhRnWGPw1hnuAomQepgJX/miTsn tvugR6g4zKSyJClZGuUYbLf22sAA670HSeUxZdz4tG5GuMxNuGoulw6FD5LWwjdeEcpy g5Ro1d9zYhLqldUasF0CUq0mdnLSfjF5ckrxurng01AbsOgt/gUfu1QiIXF5XpZDNgg1 OgUmESs2N1NQy8EDZoNHojT406z4LpLDrVTqQqFUZt5SQRYNzy55rMj2/ttsI/CvYSRa G07w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702389619; x=1702994419; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=u3GNzdEDqQxiCOIUcXvEz+JlsRFHHXMx8I/HEqc6Pqw=; b=qfwWrhecqYx73F4xX4ktD+yYNSVV7VpiWUhLT7bBsR2WJvLXHSi5SaVROV7efoUwbr SAem1z9vNqac7nFUX2ROqHsMs8UUAPEsCcf0fu89IzH10Qd3iACl6ANDgalIspm4SrPl MhtJwzc39WqvCo7PP4dD7Q/qF3Ok+kczuCrMKHr4evpBhQ1cACCLQ4YQ1/ergqnfWfDG hWLpkTmV2xMKN7uhKrQciBw85MwdK8XrPL1hjDgawCcoGo8hJHDz6iGdMSF8BVHc91Q7 q+Mqi6vuaMfacY8zpDF9uYWBJV29+BPjy7FeAl0FG61Twdz7O/H19vQR7KIaa/7QZelC LeAw== X-Gm-Message-State: AOJu0Yw4ZFAhg8JngHsYrogHkcKD8QZW4f11vK3pS7tOocg4Mv+quZcm BDzmbk4cCVf5zW/JdFSu8xv8BPmzHGyseGAKEdkJ0w== X-Google-Smtp-Source: AGHT+IHyWZyIBaIx+h+tfg3Z6wYhcrlD8xojeMyqs3YCM7RQAScbutvNH7CM4sivHnVTZ450j2yO9Xl0wrBhxInzOtU= X-Received: by 2002:a17:90a:b306:b0:28a:a89d:4e85 with SMTP id d6-20020a17090ab30600b0028aa89d4e85mr1356247pjr.63.1702389618542; Tue, 12 Dec 2023 06:00:18 -0800 (PST) MIME-Version: 1.0 References: <20231212120809.13996-1-jiawei@iscas.ac.cn> In-Reply-To: <20231212120809.13996-1-jiawei@iscas.ac.cn> From: =?UTF-8?Q?Christoph_M=C3=BCllner?= Date: Tue, 12 Dec 2023 15:00:07 +0100 Message-ID: Subject: Re: [PATCH v2] RISC-V: Supports RISC-V Profiles in '-march' option. To: Jiawei Cc: gcc-patches@gcc.gnu.org, kito.cheng@sifive.com, palmer@dabbelt.com, jeffreyalaw@gmail.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Tue, Dec 12, 2023 at 1:08=E2=80=AFPM Jiawei wrote: > > Supports RISC-V profiles[1] in -march option. > > Default input set the profile is before other formal extensions. > > V2: Fixes some format errors and adds code comments for parse function > Thanks for Jeff Law's review and comments. > > [1]https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc > > gcc/ChangeLog: > > * common/config/riscv/riscv-common.cc (struct riscv_profiles): > New struct. > (riscv_subset_list::parse_profiles): New function. > (riscv_subset_list::parse): New table. > * config/riscv/riscv-subset.h: New protype. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/arch-31.c: New test. > * gcc.target/riscv/arch-32.c: New test. > * gcc.target/riscv/arch-33.c: New test. > * gcc.target/riscv/arch-34.c: New test. For the positive tests (-31.c and -33.c) it would be great to test if the enabled extension's test macros are set. Something like this would do: #if (!(defined __riscv_zicsr) || \ !(defined __riscv_...)) #error "Feature macros not defined" #endif Also, positive tests for RVI20U32 and RVI20U64 would be nice. > > --- > gcc/common/config/riscv/riscv-common.cc | 83 +++++++++++++++++++++++- > gcc/config/riscv/riscv-subset.h | 2 + > gcc/testsuite/gcc.target/riscv/arch-31.c | 5 ++ > gcc/testsuite/gcc.target/riscv/arch-32.c | 5 ++ > gcc/testsuite/gcc.target/riscv/arch-33.c | 5 ++ > gcc/testsuite/gcc.target/riscv/arch-34.c | 7 ++ > 6 files changed, 106 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/arch-31.c > create mode 100644 gcc/testsuite/gcc.target/riscv/arch-32.c > create mode 100644 gcc/testsuite/gcc.target/riscv/arch-33.c > create mode 100644 gcc/testsuite/gcc.target/riscv/arch-34.c > > diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/= riscv/riscv-common.cc > index 4d5a2f874a2..8b674a4a280 100644 > --- a/gcc/common/config/riscv/riscv-common.cc > +++ b/gcc/common/config/riscv/riscv-common.cc > @@ -195,6 +195,12 @@ struct riscv_ext_version > int minor_version; > }; > > +struct riscv_profiles > +{ > + const char *profile_name; > + const char *profile_string; > +}; > + > /* All standard extensions defined in all supported ISA spec. */ > static const struct riscv_ext_version riscv_ext_version_table[] =3D > { > @@ -379,6 +385,42 @@ static const struct riscv_ext_version riscv_combine_= info[] =3D > {NULL, ISA_SPEC_CLASS_NONE, 0, 0} > }; > > +/* This table records the mapping form RISC-V Profiles into march string= . */ > +static const riscv_profiles riscv_profiles_table[] =3D > +{ > + /* RVI20U only contains the base extesnion 'i' as mandatory extension.= */ > + {"RVI20U64", "rv64i"}, > + {"RVI20U32", "rv32i"}, > + > + /* RVA20U contains the 'i,m,a,f,d,c,zicsr' as mandatory extensions. > + Currently we don't have zicntr,ziccif,ziccrse,ziccamoa, > + zicclsm,za128rs yet. */ > + {"RVA20U64", "rv64imafdc_zicsr"}, > + > + /* RVA20S64 mandatory include all the extensions in RVA20U64 and > + additonal 'zifencei' as mandatory extensions. > + Notes that ss1p11, svbare, sv39, svade, sscptr, ssvecd, sstvala sho= uld > + control by binutils. */ > + {"RVA20S64", "rv64imafdc_zicsr_zifencei"}, > + > + /* RVA22U contains the 'i,m,a,f,d,c,zicsr,zihintpause,zba,zbb,zbs, > + zicbom,zicbop,zicboz,zfhmin,zkt' as mandatory extensions. > + Currently we don't have zicntr,zihpm,ziccif,ziccrse,ziccamoa, > + zicclsm,zic64b,za64rs yet. */ I would prefer that we implement the missing extensions that start with 'z' as "dummy" extensions. I.e., they (currently?) don't affect code generation, but they will be passed on to the assembler and will become part of the Tag_RISCV_arch string. I admit that such "dummy" extensions may not be preferred by maintainers, but we already have precedence with Zkt. I consider an incomplete expansion of a profile as misleading. And later changes to complete the expansion could be called out as "breaking changes". > + {"RVA22U64", "rv64imafdc_zicsr_zihintpause_zba_zbb_zbs" = \ > + "_zicbom_zicbop_zicboz_zfhmin_zkt"}, > + > + /* RVA22S64 mandatory include all the extensions in RVA22U64 and > + additonal 'zifencei,svpbmt,svinval' as mandatory extensions. > + Notes that ss1p12, svbare, sv39, svade, sscptr, ssvecd, sstvala, > + scounterenw extentions should control by binutils. */ Typo: extentions -> extensions I want to challenge the implementation of RVA22S64 support (or in general all S-mode and M-mode profile support) in toolchains: * Adding 's*'/'m*' extensions as dummy extensions won't have much use * Having an incomplete extension is misleading (see above) * I doubt that RVA22S64 would find many users Therefore, I would not add support for S-mode and M-mode profiles. > + {"RVA22S64","rv64imafdc_zicsr_zifencei_zihintpause" = \ > + "_zba_zbb_zbs_zicbom_zicbop_zicboz_zfhmin_zkt_svpbmt_svinval"}, > + > + /* Terminate the list. */ > + {NULL, NULL} > +}; > + > static const riscv_cpu_info riscv_cpu_tables[] =3D > { > #define RISCV_CORE(CORE_NAME, ARCH, TUNE) \ > @@ -958,6 +1000,42 @@ riscv_subset_list::parsing_subset_version (const ch= ar *ext, > return p; > } > > +/* Parsing RISC-V Profiles in -march string. > + Return string with mandatory extensions of Profiles. */ > +const char * > +riscv_subset_list::parse_profiles (const char * p){ > + /* Checking if input string contains a Profiles. > + There are two cases use Proifles in -march option > + > + 1. Only use Proifles as -march input > + 2. Mixed Profiles with other extensions > + > + use '+' to split Profiles and other extension. */ > + for (int i =3D 0; riscv_profiles_table[i].profile_name !=3D NULL; ++i)= { > + const char* match =3D strstr(p, riscv_profiles_table[i].profile_name= ); > + const char* plus_ext =3D strchr(p, '+'); > + /* Find profile at the begin. */ > + if (match !=3D NULL && match =3D=3D p) { > + /* If there's no '+' sign, return the profile_string directly. */ > + if(!plus_ext) > + return riscv_profiles_table[i].profile_string; > + /* If there's a '+' sign, need to add profiles with other ext. */ > + else { > + size_t arch_len =3D strlen(riscv_profiles_table[i].profile_string= )+ > + strlen(plus_ext); > + /* Reset the input string with Profiles mandatory extensions, > + end with '_' to connect other additional extensions. */ > + static char* result =3D new char[arch_len + 2]; > + strcpy(result, riscv_profiles_table[i].profile_string); > + strcat(result, "_"); > + strcat(result, plus_ext + 1); /* skip the '+'. */ > + return result; > + } > + } > + } > + return p; > +} > + > /* Parsing function for standard extensions. > > Return Value: > @@ -1486,7 +1564,10 @@ riscv_subset_list::parse (const char *arch, locati= on_t loc) > > riscv_subset_list *subset_list =3D new riscv_subset_list (arch, loc); > riscv_subset_t *itr; > + > const char *p =3D arch; > + p =3D subset_list->parse_profiles(p); > + > if (startswith (p, "rv32")) > { > subset_list->m_xlen =3D 32; > @@ -1499,7 +1580,7 @@ riscv_subset_list::parse (const char *arch, locatio= n_t loc) > } > else > { > - error_at (loc, "%<-march=3D%s%>: ISA string must begin with rv32 o= r rv64", > + error_at (loc, "%<-march=3D%s%>: ISA string must begin with rv32, = rv64 or a profile", > arch); > goto fail; > } > diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-sub= set.h > index ad1cab2aa24..ba991f67014 100644 > --- a/gcc/config/riscv/riscv-subset.h > +++ b/gcc/config/riscv/riscv-subset.h > @@ -76,6 +76,8 @@ private: > const char *parse_single_multiletter_ext (const char *, const char *, > const char *); > > + const char *parse_profiles (const char*); > + > void handle_implied_ext (const char *); > bool check_implied_ext (); > void handle_combine_ext (); > diff --git a/gcc/testsuite/gcc.target/riscv/arch-31.c b/gcc/testsuite/gcc= .target/riscv/arch-31.c > new file mode 100644 > index 00000000000..eb24abe4153 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/arch-31.c > @@ -0,0 +1,5 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=3DRVI20U64 -mabi=3Dlp64" } */ > +int foo() > +{ > +} > diff --git a/gcc/testsuite/gcc.target/riscv/arch-32.c b/gcc/testsuite/gcc= .target/riscv/arch-32.c > new file mode 100644 > index 00000000000..bc556a3e717 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/arch-32.c > @@ -0,0 +1,5 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=3DRVI20U64+mafdc -mabi=3Dlp64d" } */ > +int foo() > +{ > +} > diff --git a/gcc/testsuite/gcc.target/riscv/arch-33.c b/gcc/testsuite/gcc= .target/riscv/arch-33.c > new file mode 100644 > index 00000000000..a7b80a5ad43 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/arch-33.c > @@ -0,0 +1,5 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=3DRVA22S64 -mabi=3Dlp64d" } */ > +int foo() > +{ > +} > diff --git a/gcc/testsuite/gcc.target/riscv/arch-34.c b/gcc/testsuite/gcc= .target/riscv/arch-34.c > new file mode 100644 > index 00000000000..d077aba4fc2 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/arch-34.c > @@ -0,0 +1,7 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=3Drv64gcRVA22S64 -mabi=3Dlp64d" } */ > +int foo() > +{ > +} > + > +/* { dg-error "ISA string must begin with rv32, rv64 or a profile" "" { = target *-*-* } 0 } */ > -- > 2.25.1 >