From: Palmer Dabbelt <palmer@rivosinc.com>
To: Nelson Chu <nelson.chu@sifive.com>
Cc: Nick Clifton <nickc@redhat.com>, binutils@sourceware.org
Subject: Re: [PATCH] RISC-V: Stop reporting warnings for mismatched extension versions
Date: Mon, 07 Feb 2022 18:45:55 -0800 (PST) [thread overview]
Message-ID: <mhng-2f00bbec-41b1-47e8-84cd-f7c801e90f7e@palmer-ri-x1c9> (raw)
In-Reply-To: <CAJYME4Host+CdtqjLdkwxwELXxbwWL4aFgLfwDo+geUsqHWmeQ@mail.gmail.com>
On Mon, 07 Feb 2022 18:03:37 PST (-0800), Nelson Chu wrote:
> 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)
Kito points out that !true is actually false, not true. I'll send a v2,
after making sure it works...
>> 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:45 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
2022-02-08 2:45 ` Palmer Dabbelt [this message]
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=mhng-2f00bbec-41b1-47e8-84cd-f7c801e90f7e@palmer-ri-x1c9 \
--to=palmer@rivosinc.com \
--cc=binutils@sourceware.org \
--cc=nelson.chu@sifive.com \
--cc=nickc@redhat.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).