From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x92a.google.com (mail-ua1-x92a.google.com [IPv6:2607:f8b0:4864:20::92a]) by sourceware.org (Postfix) with ESMTPS id 29DF83858D1E for ; Thu, 22 Dec 2022 09:38:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 29DF83858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ua1-x92a.google.com with SMTP id n9so264260uao.13 for ; Thu, 22 Dec 2022 01:38:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=GTsir9H8PLHsqTMluO8VhlKFGGg0UsTMLl13Ve98yMQ=; b=gjblksBG+Zff/KrjxxRDmfn6wNmz150I+QC3rVAbY9S11/qJUCdNF08WOzX+c/nWC5 MFa+PjD0ro+W8L0w86tzl3U4SSRyYSagGU2S7T9rnA6E7DkeOMJtbFg3rXSVV2ijYKnZ LwIllT4QnHpVr/8EzRl7wk9y2NlMwnkNPHMf1LtBdT/vpoTFKRAYHg/I1sRATtjQ17ok 9J81qN3EM9h7rLZuj281XR1uH7cdNQ3xB4l7plc4Vpme0Vw4QwuMD1/HipqKlCAwY7ei UX/e6QITCMc+V1n1Vu2FKgm7lYrlXLcjB3rVaqqaSXGSZDQUOGPAADNSnKCU+wU+dtZi PNKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=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=GTsir9H8PLHsqTMluO8VhlKFGGg0UsTMLl13Ve98yMQ=; b=06Nln5ISMV/7RrHIkEqBGjg7NCGNnt2tt1pNFM51qtEsoMib78ZJa5TvCMdVg2mrS4 MwU8J/vxJt4esPKkGQQaD3gW+8VtW4hXyG9c53Mq/p1MjfE+aT5Qdp2qnIktpZnlLsk6 WUOW/tZK/wFpuU4fE0KIijrnF1Dt3JCR36yH5Yl8jxTcT6FFLJb46YtnstsD1B6uB0o+ 9FY8mDBuV5KSc8zv7ZScnDnJwT4GdqwdNfd2aGFMPzy+d5H1oRpMp5MOTyaNkQqM/oay O3bq5lDgc6LF6IVioW4FsGq0lATMovDl65pMMszsUtSywFv4UOQfUl2jPOey4pImoyKc vXVw== X-Gm-Message-State: AFqh2koObx9vZaPQ+loN0XnR9Kz8/mOT/ZNK3rDN5YJHorgS9DkXYvKw tRy6oTw34YXjNQAQ1L+nnC9RXwuNzph98Q/72kPOBYSm X-Google-Smtp-Source: AMrXdXuqiPLw2QOWrhzs9x1qquj+DkGJwIP2sPgEO1x/BbBpHgCmuQTeRgIjQLI6agv+kVeDFIsPHhttmGl0cl7x3Ic= X-Received: by 2002:ab0:6684:0:b0:418:890a:161b with SMTP id a4-20020ab06684000000b00418890a161bmr437340uan.71.1671701938030; Thu, 22 Dec 2022 01:38:58 -0800 (PST) MIME-Version: 1.0 References: <20221221120934.45775-1-nelson@rivosinc.com> In-Reply-To: <20221221120934.45775-1-nelson@rivosinc.com> From: Kito Cheng Date: Thu, 22 Dec 2022 17:38:46 +0800 Message-ID: Subject: Re: [PATCH] RISC-V: Relax the order checking for the architecture string. To: Nelson Chu Cc: binutils@sourceware.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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: LGTM, thanks! On Wed, Dec 21, 2022 at 8:10 PM Nelson Chu wrote: > > * riscv-toolchain-conventions, > PR, https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/14 > Issue, https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/11 > > * Refer to the commit afc41ffb, > RISC-V: Reorder the prefixed extensions which are out of order. > > In the past we only allow to reorder the prefixed extensions. But according > to the PR 14 in the riscv-toolchain-convention, we can also relax the order > checking to allow the whole extensions be written out of orders, including > the single standard extensions and the prefixed multi-letter extensions. > Just that we still need to follow the following rules as usual, > > 1. prefixed extensions need to be seperated with `_'. > 2. prefixed extensions need complete . version if set. > > Please see the details in the march-ok-reorder gas testcase. > > Passed the riscv-gnu-toolchain regressions. > > bfd/ > * elfxx-riscv.c (enum riscv_prefix_ext_class): Changed RV_ISA_CLASS_UNKNOWN > to RV_ISA_CLASS_SINGLE, since everything that does not belong to the > multi-keyword will possible be a single extension for the current parser. > (parse_config): Likewise. > (riscv_get_prefix_class): Likewise. > (riscv_compare_subsets): Likewise. > (riscv_parse_std_ext): Removed, and merged with riscv_parse_prefixed_ext > into riscv_parse_extensions. > (riscv_parse_prefixed_ext): Likewise. > (riscv_parse_subset): Only need to call riscv_parse_extensions to parse > both single standard and prefixed extensions. > gas/ > * testsuite/gas/riscv/march-fail-order-std.d: Removed since the relaxed > order checking. > * testsuite/gas/riscv/march-fail-order-std.l: Likewise. > * testsuite/gas/riscv/march-fail-order-x-std.d: Likewise. > * testsuite/gas/riscv/march-fail-order-z-std.d: Likewise. > * testsuite/gas/riscv/march-fail-order-zx-std.l: Likewise. > * testsuite/gas/riscv/march-fail-unknown-std.l: Updated. > * testsuite/gas/riscv/march-ok-reorder.d: New testcase. > --- > bfd/elfxx-riscv.c | 210 +++++++----------- > .../gas/riscv/march-fail-order-std.d | 3 - > .../gas/riscv/march-fail-order-std.l | 2 - > .../gas/riscv/march-fail-order-x-std.d | 3 - > .../gas/riscv/march-fail-order-z-std.d | 3 - > .../gas/riscv/march-fail-order-zx-std.l | 2 - > .../gas/riscv/march-fail-unknown-std.l | 2 +- > gas/testsuite/gas/riscv/march-ok-reorder.d | 7 + > 8 files changed, 83 insertions(+), 149 deletions(-) > delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-std.d > delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-std.l > delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-x-std.d > delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-z-std.d > delete mode 100644 gas/testsuite/gas/riscv/march-fail-order-zx-std.l > create mode 100644 gas/testsuite/gas/riscv/march-ok-reorder.d > > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c > index 0bcf2fdcfa3..423c67ad344 100644 > --- a/bfd/elfxx-riscv.c > +++ b/bfd/elfxx-riscv.c > @@ -1275,7 +1275,7 @@ enum riscv_prefix_ext_class > RV_ISA_CLASS_S, > RV_ISA_CLASS_ZXM, > RV_ISA_CLASS_X, > - RV_ISA_CLASS_UNKNOWN > + RV_ISA_CLASS_SINGLE > }; > > /* Record the strings of the prefixed extensions, and their corresponding > @@ -1296,7 +1296,7 @@ static const struct riscv_parse_prefix_config parse_config[] = > {RV_ISA_CLASS_Z, "z"}, > {RV_ISA_CLASS_S, "s"}, > {RV_ISA_CLASS_X, "x"}, > - {RV_ISA_CLASS_UNKNOWN, NULL} > + {RV_ISA_CLASS_SINGLE, NULL} > }; > > /* Get the prefixed name class for the extensions, the class also > @@ -1306,14 +1306,14 @@ static enum riscv_prefix_ext_class > riscv_get_prefix_class (const char *arch) > { > int i = 0; > - while (parse_config[i].class != RV_ISA_CLASS_UNKNOWN) > + while (parse_config[i].class != RV_ISA_CLASS_SINGLE) > { > if (strncmp (arch, parse_config[i].prefix, > strlen (parse_config[i].prefix)) == 0) > return parse_config[i].class; > i++; > } > - return RV_ISA_CLASS_UNKNOWN; > + return RV_ISA_CLASS_SINGLE; > } > > /* Check KNOWN_EXTS to see if the EXT is supported. */ > @@ -1405,9 +1405,9 @@ riscv_compare_subsets (const char *subset1, const char *subset2) > enum riscv_prefix_ext_class class1 = riscv_get_prefix_class (subset1); > enum riscv_prefix_ext_class class2 = riscv_get_prefix_class (subset2); > > - if (class1 != RV_ISA_CLASS_UNKNOWN) > + if (class1 != RV_ISA_CLASS_SINGLE) > order1 = - (int) class1; > - if (class2 != RV_ISA_CLASS_UNKNOWN) > + if (class2 != RV_ISA_CLASS_SINGLE) > order2 = - (int) class2; > > if (order1 == order2) > @@ -1659,7 +1659,7 @@ riscv_parsing_subset_version (const char *p, > return p; > } > > -/* Parsing function for standard extensions. > +/* Parsing function for both standard and prefixed extensions. > > Return Value: > Points to the end of extensions. > @@ -1670,9 +1670,9 @@ riscv_parsing_subset_version (const char *p, > `p`: Curent parsing position. */ > > static const char * > -riscv_parse_std_ext (riscv_parse_subset_t *rps, > - const char *arch, > - const char *p) > +riscv_parse_extensions (riscv_parse_subset_t *rps, > + const char *arch, > + const char *p) > { > /* First letter must start with i, e or g. */ > if (*p != 'e' && *p != 'i' && *p != 'g') > @@ -1683,79 +1683,7 @@ riscv_parse_std_ext (riscv_parse_subset_t *rps, > return NULL; > } > > - while (p != NULL && *p != '\0') > - { > - /* Stop when we parsed the known prefix class. */ > - enum riscv_prefix_ext_class class = riscv_get_prefix_class (p); > - if (class != RV_ISA_CLASS_UNKNOWN) > - break; > - > - if (*p == '_') > - { > - p++; > - continue; > - } > - > - bool implicit = false; > - int major = RISCV_UNKNOWN_VERSION; > - int minor = RISCV_UNKNOWN_VERSION; > - char subset[2] = {0, 0}; > - > - subset[0] = *p; > - > - /* Check if the standard extension is supported. */ > - if (riscv_ext_order[(subset[0] - 'a')] == 0) > - { > - rps->error_handler > - (_("%s: unknown standard ISA extension `%c'"), > - arch, subset[0]); > - return NULL; > - } > - > - /* Checking canonical order. */ > - if (rps->subset_list->tail != NULL > - && riscv_compare_subsets (rps->subset_list->tail->name, subset) > 0) > - { > - rps->error_handler > - (_("%s: standard ISA extension `%c' is not " > - "in canonical order"), arch, subset[0]); > - return NULL; > - } > - > - p = riscv_parsing_subset_version (++p, &major, &minor); > - /* Added g as an implicit extension. */ > - if (subset[0] == 'g') > - { > - implicit = true; > - major = RISCV_UNKNOWN_VERSION; > - minor = RISCV_UNKNOWN_VERSION; > - } > - riscv_parse_add_subset (rps, subset, major, minor, implicit); > - } > - > - return p; > -} > - > -/* Parsing function for prefixed extensions. > - > - Return Value: > - Points to the end of extension. > - > - Arguments: > - `rps`: Hooks and status for parsing extensions. > - `arch`: Full ISA string. > - `p`: Curent parsing position. */ > - > -static const char * > -riscv_parse_prefixed_ext (riscv_parse_subset_t *rps, > - const char *arch, > - const char *p) > -{ > - int major_version; > - int minor_version; > - enum riscv_prefix_ext_class class; > - > - while (*p) > + while (*p != '\0') > { > if (*p == '_') > { > @@ -1763,52 +1691,62 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps, > continue; > } > > - class = riscv_get_prefix_class (p); > - if (class == RV_ISA_CLASS_UNKNOWN) > - { > - rps->error_handler > - (_("%s: unknown prefix class for the ISA extension `%s'"), > - arch, p); > - return NULL; > - } > - > char *subset = xstrdup (p); > - char *q = subset; > + char *q = subset; /* Start of version. */ > const char *end_of_version; > + bool implicit = false; > > - /* Extract the whole prefixed extension by '_'. */ > - while (*++q != '\0' && *q != '_') > - ; > - /* Look forward to the first letter which is not p. */ > - bool find_any_version = false; > - bool find_minor_version = false; > - while (1) > + enum riscv_prefix_ext_class class = riscv_get_prefix_class (p); > + if (class == RV_ISA_CLASS_SINGLE) > { > - q--; > - if (ISDIGIT (*q)) > - find_any_version = true; > - else if (find_any_version > - && !find_minor_version > - && *q == 'p' > - && ISDIGIT (*(q - 1))) > - find_minor_version = true; > - else > - break; > + if (riscv_ext_order[(*subset - 'a')] == 0) > + { > + rps->error_handler > + (_("%s: unknown standard ISA extension or prefix class `%c'"), > + arch, *subset); > + free (subset); > + return NULL; > + } > + q++; > } > - q++; > - > - /* Check if the end of extension is 'p' or not. If yes, then > - the second letter from the end cannot be number. */ > - if (*(q - 1) == 'p' && ISDIGIT (*(q - 2))) > + else > { > - *q = '\0'; > - rps->error_handler > - (_("%s: invalid prefixed ISA extension `%s' ends with p"), > - arch, subset); > - free (subset); > - return NULL; > + /* Extract the whole prefixed extension by '_'. */ > + while (*++q != '\0' && *q != '_') > + ; > + /* Look forward to the first letter which is not p. */ > + bool find_any_version = false; > + bool find_minor_version = false; > + while (1) > + { > + q--; > + if (ISDIGIT (*q)) > + find_any_version = true; > + else if (find_any_version > + && !find_minor_version > + && *q == 'p' > + && ISDIGIT (*(q - 1))) > + find_minor_version = true; > + else > + break; > + } > + q++; > + > + /* Check if the end of extension is 'p' or not. If yes, then > + the second letter from the end cannot be number. */ > + if (*(q - 1) == 'p' && ISDIGIT (*(q - 2))) > + { > + *q = '\0'; > + rps->error_handler > + (_("%s: invalid prefixed ISA extension `%s' ends with p"), > + arch, subset); > + free (subset); > + return NULL; > + } > } > > + int major_version = RISCV_UNKNOWN_VERSION; > + int minor_version = RISCV_UNKNOWN_VERSION; > end_of_version = > riscv_parsing_subset_version (q, &major_version, &minor_version); > *q = '\0'; > @@ -1818,8 +1756,9 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps, > return NULL; > } > > - /* Check that the extension name is well-formed. */ > - if (rps->check_unknown_prefixed_ext > + /* Check if the prefixed extension name is well-formed. */ > + if (class != RV_ISA_CLASS_SINGLE > + && rps->check_unknown_prefixed_ext > && !riscv_recognized_prefixed_ext (subset)) > { > rps->error_handler > @@ -1829,13 +1768,22 @@ riscv_parse_prefixed_ext (riscv_parse_subset_t *rps, > return NULL; > } > > + /* Added g as an implicit extension. */ > + if (class == RV_ISA_CLASS_SINGLE > + && strcmp (subset, "g") == 0) > + { > + implicit = true; > + major_version = RISCV_UNKNOWN_VERSION; > + minor_version = RISCV_UNKNOWN_VERSION; > + } > riscv_parse_add_subset (rps, subset, > major_version, > - minor_version, false); > + minor_version, implicit); > p += end_of_version - subset; > free (subset); > > - if (*p != '\0' && *p != '_') > + if (class != RV_ISA_CLASS_SINGLE > + && *p != '\0' && *p != '_') > { > rps->error_handler > (_("%s: prefixed ISA extension must separate with _"), > @@ -2008,16 +1956,8 @@ riscv_parse_subset (riscv_parse_subset_t *rps, > return false; > } > > - /* Parsing standard extension. */ > - p = riscv_parse_std_ext (rps, arch, p); > - > - if (p == NULL) > - return false; > - > - /* Parse prefixed extensions. */ > - p = riscv_parse_prefixed_ext (rps, arch, p); > - > - if (p == NULL) > + /* Parse single standard and prefixed extensions. */ > + if (riscv_parse_extensions (rps, arch, p) == NULL) > return false; > > /* Finally add implicit extensions according to the current > diff --git a/gas/testsuite/gas/riscv/march-fail-order-std.d b/gas/testsuite/gas/riscv/march-fail-order-std.d > deleted file mode 100644 > index b9c7e09de1a..00000000000 > --- a/gas/testsuite/gas/riscv/march-fail-order-std.d > +++ /dev/null > @@ -1,3 +0,0 @@ > -#as: -march=rv32iamfd > -#source: empty.s > -#error_output: march-fail-order-std.l > diff --git a/gas/testsuite/gas/riscv/march-fail-order-std.l b/gas/testsuite/gas/riscv/march-fail-order-std.l > deleted file mode 100644 > index 9e3ce5e8d91..00000000000 > --- a/gas/testsuite/gas/riscv/march-fail-order-std.l > +++ /dev/null > @@ -1,2 +0,0 @@ > -.*Assembler messages: > -.*Error: .*standard ISA extension `m' is not in canonical order > diff --git a/gas/testsuite/gas/riscv/march-fail-order-x-std.d b/gas/testsuite/gas/riscv/march-fail-order-x-std.d > deleted file mode 100644 > index 4762f3dd95c..00000000000 > --- a/gas/testsuite/gas/riscv/march-fail-order-x-std.d > +++ /dev/null > @@ -1,3 +0,0 @@ > -#as: -march=rv32i_xargle2p0_mafd > -#source: empty.s > -#error_output: march-fail-order-zx-std.l > diff --git a/gas/testsuite/gas/riscv/march-fail-order-z-std.d b/gas/testsuite/gas/riscv/march-fail-order-z-std.d > deleted file mode 100644 > index 42526de3621..00000000000 > --- a/gas/testsuite/gas/riscv/march-fail-order-z-std.d > +++ /dev/null > @@ -1,3 +0,0 @@ > -#as: -march=rv32i_zicsr2p0_mafd > -#source: empty.s > -#error_output: march-fail-order-zx-std.l > diff --git a/gas/testsuite/gas/riscv/march-fail-order-zx-std.l b/gas/testsuite/gas/riscv/march-fail-order-zx-std.l > deleted file mode 100644 > index 4f6b98c592e..00000000000 > --- a/gas/testsuite/gas/riscv/march-fail-order-zx-std.l > +++ /dev/null > @@ -1,2 +0,0 @@ > -.*Assembler messages: > -.*Error: .*unknown prefix class for the ISA extension `mafd' > diff --git a/gas/testsuite/gas/riscv/march-fail-unknown-std.l b/gas/testsuite/gas/riscv/march-fail-unknown-std.l > index 834a4857c58..1de7a420e9b 100644 > --- a/gas/testsuite/gas/riscv/march-fail-unknown-std.l > +++ b/gas/testsuite/gas/riscv/march-fail-unknown-std.l > @@ -1,2 +1,2 @@ > .*Assembler messages: > -.*Error: .*unknown standard ISA extension `y' > +.*Error: .*unknown standard ISA extension or prefix class `y' > diff --git a/gas/testsuite/gas/riscv/march-ok-reorder.d b/gas/testsuite/gas/riscv/march-ok-reorder.d > new file mode 100644 > index 00000000000..030f8b15018 > --- /dev/null > +++ b/gas/testsuite/gas/riscv/march-ok-reorder.d > @@ -0,0 +1,7 @@ > +#as: -misa-spec=20191213 -march=rv32i2azicsr_fc2p0dxfoo2p0_m1_xbar2p0_zba > +#source: empty.s > +#readelf: -A > + > +Attribute Section: riscv > +File Attributes > + Tag_RISCV_arch: "rv32i2p0_m1p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zmmul1p0_zba1p0_xbar2p0_xfoo2p0" > -- > 2.37.1 (Apple Git-137.1) >