From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x32.google.com (mail-oa1-x32.google.com [IPv6:2001:4860:4864:20::32]) by sourceware.org (Postfix) with ESMTPS id 891143858D1E for ; Fri, 23 Dec 2022 02:05:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 891143858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com Received: by mail-oa1-x32.google.com with SMTP id 586e51a60fabf-144b21f5e5fso4549553fac.12 for ; Thu, 22 Dec 2022 18:05:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20210112.gappssmtp.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=/5PFbnIgKaQmuKxdpIenRUf91/wAXXU26rx6s9UBrtQ=; b=VWX8sGV4XCbJrjiwnIj2i9dQa6h4v8BIWzlE+wG77ncTK4/2Oma5O+3C9AQEyVGtT7 p9HVH7mcGIOFVrB9Hu9TMALuA693EfK4VIfA73Xwk676kLDchW4bvmL/tzzkDTmtEaKb E+XWfDyz5sRDPqSEXSqIYB6tdKmQ1b5TILsNKEYY+s4o5cMG/DhBHCpJaL1XBY0m8dwb o1t4J9+U5LGUSGLGuNpLthe9WoqwfR4/DGLhvKsAbPI7WmF7IGC0U0eetkK+JNzNs2JP jJKr+WCVFOlE6t0zvTbJJhEazOzAYu/eIDEd8m0hKKcUJZHFbZZDyVV4nu9cVmG+r0Si /X5g== 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=/5PFbnIgKaQmuKxdpIenRUf91/wAXXU26rx6s9UBrtQ=; b=i7HwA+7j6J/jAd6Hz4+cavxcHPbQyfv7ZHkxqLps18f6NDykK7HSqF4sb3rfY+qVm1 6o48/OyQi2ceAYaXw1XE86SLh478gurabM7aHzWUgb9Zrq0ENNojGM3JgTSGuis+5Tr+ IHO5VL7Elgmzrs01yeAC4PqUUOcFgBHZBGUCjnhvamGOSPlRDBG+WL+PAZFQ7Ao4Dtg0 uzqxP85UeLu8rrTZEgqD8pIQ6BC/PjbkCcCZpCMmTMgjXuDb/RPwz8IF5VQdu73uGhd1 uCPCc0ilAy84hHwChGbWrpkkty8oYfdYzcSuDODhbTwFD/BZZWoDAGZmGf3I1fJ3IPlE ZujA== X-Gm-Message-State: AFqh2krwh3stTuAk3E9lXFNSMYiLePjYOu506kofTh4jYVLXCTq2q1sE F8j7gCn+wFurJgF5TVPpbkStfnOJKNodzsufRyncDQ== X-Google-Smtp-Source: AMrXdXsJEXQTxBkaIqaKIRsb6osnsqgd1YtMotWeRGpreHiW3RAAK6PyCLzDKFTgYIYdtQDCVck8dnp6yfQTz/wU+wU= X-Received: by 2002:a05:6870:be95:b0:144:543:c801 with SMTP id nx21-20020a056870be9500b001440543c801mr629057oab.201.1671761153780; Thu, 22 Dec 2022 18:05:53 -0800 (PST) MIME-Version: 1.0 References: <20221221120934.45775-1-nelson@rivosinc.com> In-Reply-To: From: Nelson Chu Date: Fri, 23 Dec 2022 10:05:42 +0800 Message-ID: Subject: Re: [PATCH] RISC-V: Relax the order checking for the architecture string. To: Kito Cheng Cc: binutils@sourceware.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,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: Thanks! Committed. Nelson On Thu, Dec 22, 2022 at 5:38 PM Kito Cheng wrote: > > 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) > >