From: Nelson Chu <nelson.chu@sifive.com>
To: Palmer Dabbelt <palmer@rivosinc.com>, Nick Clifton <nickc@redhat.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [PATCH] RISC-V: Stop reporting warnings for mismatched extension versions
Date: Tue, 8 Feb 2022 10:03:37 +0800 [thread overview]
Message-ID: <CAJYME4Host+CdtqjLdkwxwELXxbwWL4aFgLfwDo+geUsqHWmeQ@mail.gmail.com> (raw)
In-Reply-To: <20220208015928.14116-1-palmer@rivosinc.com>
Hi Palmer,
LGTM, thanks!
Hi Nick,
Could we commit this into 2.38 branch? Will it be too late?
Thanks
Nelson
On Tue, Feb 8, 2022 at 10:00 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> The extension version checking logic is really just too complicated to
> encode into the linker, trying to do so causes more harm than good.
> This removes the checks and the associated tests, leaving the logic to
> keep the largest version of each extension linked into the target.
>
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> bfd/
>
> * elfnn-riscv.c (riscv_version_mismatch): Rename to
> riscv_update_subset_version, and stop reporting warnings on
> version mismatches.
> (riscv_merge_std_ext): Adjust calls to riscv_version_mismatch.
> (riscv_merge_multi_letter_ext): Likewise.
>
> ld/
> * testsuite/ld-riscv-elf/attr-merge-arch-failed-01.d: Remove
> * testsuite/ld-riscv-elf/attr-merge-arch-failed-01a.s: Likewise
> * testsuite/ld-riscv-elf/attr-merge-arch-failed-01b.s: Likewise
> * testsuite/ld-riscv-elf/attr-merge-arch-failed-02.d: Likewise
> * testsuite/ld-riscv-elf/attr-merge-arch-failed-02a.s: Likewise
> * testsuite/ld-riscv-elf/attr-merge-arch-failed-02b.s: Likewise
> * testsuite/ld-riscv-elf/attr-merge-arch-failed-02c.s: Likewise
> * testsuite/ld-riscv-elf/attr-merge-arch-failed-02d.s: Likewise
> * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Remove obselete
> attr-merge-arch-failed-{01,02}.
> ---
> bfd/elfnn-riscv.c | 77 ++++++-------------
> .../ld-riscv-elf/attr-merge-arch-failed-01.d | 11 ---
> .../ld-riscv-elf/attr-merge-arch-failed-01a.s | 1 -
> .../ld-riscv-elf/attr-merge-arch-failed-01b.s | 1 -
> .../ld-riscv-elf/attr-merge-arch-failed-02.d | 27 -------
> .../ld-riscv-elf/attr-merge-arch-failed-02a.s | 1 -
> .../ld-riscv-elf/attr-merge-arch-failed-02b.s | 1 -
> .../ld-riscv-elf/attr-merge-arch-failed-02c.s | 1 -
> .../ld-riscv-elf/attr-merge-arch-failed-02d.s | 1 -
> ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 2 -
> 10 files changed, 24 insertions(+), 99 deletions(-)
> delete mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01.d
> delete mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01a.s
> delete mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01b.s
> delete mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02.d
> delete mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02a.s
> delete mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02b.s
> delete mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02c.s
> delete mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02d.s
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 95fcc77bea..d3b434c85f 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -3371,52 +3371,26 @@ riscv_std_ext_p (const char *name)
> return (strlen (name) == 1) && (name[0] != 'x') && (name[0] != 's');
> }
>
> -/* Check if the versions are compatible. */
> +/* Update the output subset's version to match the input when the input
> + subset's version is newer. */
>
> -static bool
> -riscv_version_mismatch (bfd *ibfd,
> - struct riscv_subset_t *in,
> - struct riscv_subset_t *out)
> +static void
> +riscv_update_subset_version (struct riscv_subset_t *in,
> + struct riscv_subset_t *out)
> {
> if (in == NULL || out == NULL)
> - return true;
> + return;
>
> - /* Since there are no version conflicts for now, we just report
> - warning when the versions are mis-matched. */
> - if (in->major_version != out->major_version
> - || in->minor_version != out->minor_version)
> + /* Update the output ISA versions to the newest ones, but otherwise don't
> + provide any errors or warnings about mis-matched ISA versions as it's
> + generally too tricky to check for these at link time. */
> + if ((in->major_version > out->major_version)
> + || (in->major_version == out->major_version
> + && in->minor_version > out->minor_version))
> {
> - if ((in->major_version == RISCV_UNKNOWN_VERSION
> - && in->minor_version == RISCV_UNKNOWN_VERSION)
> - || (out->major_version == RISCV_UNKNOWN_VERSION
> - && out->minor_version == RISCV_UNKNOWN_VERSION))
> - {
> - /* Do not report the warning when the version of input
> - or output is RISCV_UNKNOWN_VERSION, since the extension
> - is added implicitly. */
> - }
> - else
> - _bfd_error_handler
> - (_("warning: %pB: mis-matched ISA version %d.%d for '%s' "
> - "extension, the output version is %d.%d"),
> - ibfd,
> - in->major_version,
> - in->minor_version,
> - in->name,
> - out->major_version,
> - out->minor_version);
> -
> - /* Update the output ISA versions to the newest ones. */
> - if ((in->major_version > out->major_version)
> - || (in->major_version == out->major_version
> - && in->minor_version > out->minor_version))
> - {
> - out->major_version = in->major_version;
> - out->minor_version = in->minor_version;
> - }
> + out->major_version = in->major_version;
> + out->minor_version = in->minor_version;
> }
> -
> - return true;
> }
>
> /* Return true if subset is 'i' or 'e'. */
> @@ -3477,11 +3451,10 @@ riscv_merge_std_ext (bfd *ibfd,
> ibfd, in->name, out->name);
> return false;
> }
> - else if (!riscv_version_mismatch (ibfd, in, out))
> - return false;
> - else
> - riscv_add_subset (&merged_subsets,
> - out->name, out->major_version, out->minor_version);
> +
> + riscv_update_subset_version(in, out);
> + riscv_add_subset (&merged_subsets,
> + out->name, out->major_version, out->minor_version);
>
> in = in->next;
> out = out->next;
> @@ -3499,11 +3472,11 @@ riscv_merge_std_ext (bfd *ibfd,
> if (!find_in && !find_out)
> continue;
>
> - if (find_in
> - && find_out
> - && !riscv_version_mismatch (ibfd, ext_in, ext_out))
> + if (find_in && find_out)
> return false;
>
> + riscv_update_subset_version(ext_in, ext_out);
> +
> ext_merged = find_out ? ext_out : ext_in;
> riscv_add_subset (&merged_subsets, ext_merged->name,
> ext_merged->major_version, ext_merged->minor_version);
> @@ -3524,8 +3497,7 @@ riscv_merge_std_ext (bfd *ibfd,
> on success and FALSE when a conflict is found. */
>
> static bool
> -riscv_merge_multi_letter_ext (bfd *ibfd,
> - riscv_subset_t **pin,
> +riscv_merge_multi_letter_ext (riscv_subset_t **pin,
> riscv_subset_t **pout)
> {
> riscv_subset_t *in = *pin;
> @@ -3555,8 +3527,7 @@ riscv_merge_multi_letter_ext (bfd *ibfd,
> else
> {
> /* Both present, check version and increment both. */
> - if (!riscv_version_mismatch (ibfd, in, out))
> - return false;
> + riscv_update_subset_version (in, out);
>
> riscv_add_subset (&merged_subsets, out->name, out->major_version,
> out->minor_version);
> @@ -3629,7 +3600,7 @@ riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch)
> return NULL;
>
> /* Merge all non-single letter extensions with single call. */
> - if (!riscv_merge_multi_letter_ext (ibfd, &in, &out))
> + if (!riscv_merge_multi_letter_ext (&in, &out))
> return NULL;
>
> if (xlen_in != xlen_out)
> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01.d b/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01.d
> deleted file mode 100644
> index 669a139206..0000000000
> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01.d
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -#source: attr-merge-arch-failed-01a.s
> -#source: attr-merge-arch-failed-01b.s
> -#as: -march-attr
> -#ld: -r -m[riscv_choose_ilp32_emul]
> -#warning: .*mis-matched ISA version 3.0 for 'a' extension, the output version is 2.0
> -#readelf: -A
> -
> -Attribute Section: riscv
> -File Attributes
> - Tag_RISCV_arch: ".*a3p0.*"
> -#..
> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01a.s b/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01a.s
> deleted file mode 100644
> index 365901d8dc..0000000000
> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01a.s
> +++ /dev/null
> @@ -1 +0,0 @@
> - .attribute arch, "rv32i2p0_m2p0_a2p0"
> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01b.s b/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01b.s
> deleted file mode 100644
> index 49263bf66d..0000000000
> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-01b.s
> +++ /dev/null
> @@ -1 +0,0 @@
> - .attribute arch, "rv32i2p0_m2p0_a3p0"
> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02.d b/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02.d
> deleted file mode 100644
> index 2f2638ace2..0000000000
> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02.d
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -#source: attr-merge-arch-failed-02a.s
> -#source: attr-merge-arch-failed-02b.s
> -#source: attr-merge-arch-failed-02c.s
> -#source: attr-merge-arch-failed-02d.s
> -#as: -march-attr
> -#ld: -r -m[riscv_choose_ilp32_emul]
> -#warning: .*mis-matched ISA version 3.0 for 'i' extension, the output version is 2.0
> -#warning: .*mis-matched ISA version 3.0 for 'm' extension, the output version is 2.0
> -#warning: .*mis-matched ISA version 3.0 for 'a' extension, the output version is 2.0
> -#warning: .*mis-matched ISA version 3.0 for 'zicsr' extension, the output version is 2.0
> -#warning: .*mis-matched ISA version 3.0 for 'xunknown' extension, the output version is 2.0
> -#warning: .*mis-matched ISA version 2.1 for 'i' extension, the output version is 3.0
> -#warning: .*mis-matched ISA version 2.2 for 'm' extension, the output version is 3.0
> -#warning: .*mis-matched ISA version 2.3 for 'a' extension, the output version is 3.0
> -#warning: .*mis-matched ISA version 2.4 for 'zicsr' extension, the output version is 3.0
> -#warning: .*mis-matched ISA version 2.5 for 'xunknown' extension, the output version is 3.0
> -#warning: .*mis-matched ISA version 4.6 for 'i' extension, the output version is 3.0
> -#warning: .*mis-matched ISA version 4.7 for 'm' extension, the output version is 3.0
> -#warning: .*mis-matched ISA version 4.8 for 'a' extension, the output version is 3.0
> -#warning: .*mis-matched ISA version 4.9 for 'zicsr' extension, the output version is 3.0
> -#warning: .*mis-matched ISA version 4.0 for 'xunknown' extension, the output version is 3.0
> -#readelf: -A
> -
> -Attribute Section: riscv
> -File Attributes
> - Tag_RISCV_arch: "rv32i4p6_m4p7_a4p8_zicsr4p9_zifencei2p0_xunknown4p0"
> -#..
> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02a.s b/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02a.s
> deleted file mode 100644
> index 3dbf8a20d5..0000000000
> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02a.s
> +++ /dev/null
> @@ -1 +0,0 @@
> - .attribute arch, "rv32i2p0_m2p0_a2p0_zicsr2p0_xunknown2p0"
> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02b.s b/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02b.s
> deleted file mode 100644
> index 7bbc39f425..0000000000
> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02b.s
> +++ /dev/null
> @@ -1 +0,0 @@
> - .attribute arch, "rv32i3p0_m3p0_a3p0_zicsr3p0_xunknown3p0"
> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02c.s b/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02c.s
> deleted file mode 100644
> index 2a921e62dc..0000000000
> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02c.s
> +++ /dev/null
> @@ -1 +0,0 @@
> - .attribute arch, "rv32i2p1_m2p2_a2p3_zicsr2p4_xunknown2p5"
> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02d.s b/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02d.s
> deleted file mode 100644
> index 6ef5ee5851..0000000000
> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-failed-02d.s
> +++ /dev/null
> @@ -1 +0,0 @@
> - .attribute arch, "rv32i4p6_m4p7_a4p8_zicsr4p9_xunknown4p0"
> diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> index 3d8950238a..0ef38fa6a4 100644
> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> @@ -160,8 +160,6 @@ if [istarget "riscv*-*-*"] {
> run_dump_test "attr-merge-priv-spec-01"
> run_dump_test "attr-merge-priv-spec-02"
> run_dump_test "attr-merge-priv-spec-03"
> - run_dump_test "attr-merge-arch-failed-01"
> - run_dump_test "attr-merge-arch-failed-02"
> run_dump_test "attr-merge-stack-align-failed"
> run_dump_test "attr-merge-priv-spec-failed-01"
> run_dump_test "attr-merge-priv-spec-failed-02"
> --
> 2.34.1
>
next prev parent reply other threads:[~2022-02-08 2:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-08 1:59 Palmer Dabbelt
2022-02-08 2:03 ` Nelson Chu [this message]
2022-02-08 2:45 ` Palmer Dabbelt
2022-02-08 13:26 ` Nick Clifton
2022-02-08 16:43 ` Palmer Dabbelt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJYME4Host+CdtqjLdkwxwELXxbwWL4aFgLfwDo+geUsqHWmeQ@mail.gmail.com \
--to=nelson.chu@sifive.com \
--cc=binutils@sourceware.org \
--cc=nickc@redhat.com \
--cc=palmer@rivosinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).