From: Nelson Chu <nelson@rivosinc.com>
To: Tsukasa OI <research_trasio@irq.a4lg.com>
Cc: "H . Peter Anvin" <hpa@zytor.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Andrew Waterman <andrew@sifive.com>,
Jim Wilson <jim.wilson.gcc@gmail.com>,
binutils@sourceware.org
Subject: Re: [PATCH v8 0/7] RISC-V: Fix address printer on the disassembler
Date: Fri, 2 Sep 2022 14:18:25 +0800 [thread overview]
Message-ID: <CAPpQWtA+9AcmymFbYFFz8oWsd3AdXtNHwu8GUM0HNg0U=7qhog@mail.gmail.com> (raw)
In-Reply-To: <cover.1661559056.git.research_trasio@irq.a4lg.com>
Thanks, committed patches 1-5 for pr29342.
Nelson
On Sat, Aug 27, 2022 at 8:11 AM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> Hello,
>
> Tracker on GitHub:
> <https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_fix_addr>
>
> Original Issue as reported by H. Peter Anvin:
> <https://sourceware.org/bugzilla/show_bug.cgi?id=29342>
>
> Previous Information on v1...v7:
> <https://sourceware.org/pipermail/binutils/2022-August/122552.html>
> See this e-mail for background.
>
>
>
> Based on Nelson's feedback, I functionally splitted the patchset to four
> mostly independent parts. Tests are self-contained to each part (unlike
> dis-addr-2.s in PATCH v7) so that we can review each part independently.
>
> (1) [PATCH 1-1] Additional tests for existing part: sequence with ADDIW
> (because we will test other cases in detail on other parts)
> (2) [PATCH 2-3] Fix to PR29342 and a new JALR issue
> (both discovered by H. Peter Anvin)
> (3) [PATCH 4-5] Make the disassembler able to print the highest address
> and GP-relative addresses even if the GP is the top addr
> (4) [PATCH 6-7] Code Tidying: Rename and retype `wide' parameter of
> `maybe_print_address' for further clarification of the code
>
>
> On 2022/08/24 20:22, Nelson Chu wrote:
> > I think there are three dis-assembler issues here,
> >
> > 1. PR29342
> > 2. The target address of jump
> > 3. Should we show the target address when it is -1
>
> cf. <https://sourceware.org/pipermail/binutils/2022-August/122573.html>
>
> Based on Nelson's classification, part (2) corresponds [1.] and [2.] and
> (3) corresponds [3.]. About issues [1.] and [2.], PATCH 2 fixes [2.] and
> PATCH 3 fixes [1.] but tests for [2.] is in the PATCH 3 (not PATCH 2).
>
> This is because PATCH 3 tests address printing in various ways including
> JALR instructions. So splitting them to PATCH 2 seemed too redundant.
>
>
> And, I couldn't come up with a good explanation to `binutils/NEWS'
> describing the changes in the part (3). Could someone help me? It enables
> printing following addresses on disassembling:
>
> - The highest address (-1)
> - GP-relative addresses if GP has the highest address (-1)
>
>
>
> [Extraction Guide]
>
> If we extract each part ((1) through (4)) using git rebase, we may
> encounter some conflicts and test failures. Even so, to extract (3) and
> (4), I recommend to apply at least (2) through (4) first and then extract
> using git rebase.
>
> (1)
> This is completely independent from the rest.
> (2)
> This is independent from (1).
> (3)
> This is independent from (1).
> This is MOSTLY independent from (2) but extracting causes one conflict
> and two test errors (that depend on the bugfix on (2)):
> # Conflict: keep those lines
> else
> return; /* Don't print the address. */
> pd->to_print_addr = true;
> # Test Failures: two changes are required to pass check-gas
> # gas/testsuite/gas/riscv/dis-addr-topaddr-32.d
> old: <addr_top>
> new: <addr_top\+0x0>
> # gas/testsuite/gas/riscv/dis-addr-topaddr-gp-32.d
> old: <addr_rel_gp_pos>
> new: <__global_pointer\$\+0x5>
> (4)
> This is independent from (1).
> This is independent from (3).
> This is MOSTLY independent from (2) but extracting causes one conflict.
> # Conflict: change like this and keep (to NOT keep the bugfix in (2))
> old: maybe_print_address (pd, rs1, 0, 0);
> new: maybe_print_address (pd, rs1, 0, false);
>
>
>
>
> Tsukasa OI (7):
> RISC-V: Add address printer tests with ADDIW
> RISC-V: Fix JALR target address computation
> RISC-V: Fix RV32 disassembler address computation
> RISC-V: Print highest address on the disassembler
> RISC-V: Print top GP-relative addresses on the disassembler
> RISC-V: Clarify that `wide' is only used for ADDIW
> RISC-V: Make `is_addiw' parameter bool
>
> gas/testsuite/gas/riscv/dis-addr-addiw-a.d | 18 +++++
> gas/testsuite/gas/riscv/dis-addr-addiw-b.d | 18 +++++
> gas/testsuite/gas/riscv/dis-addr-addiw.s | 28 ++++++++
> .../gas/riscv/dis-addr-overflow-32.d | 30 ++++++++
> .../gas/riscv/dis-addr-overflow-64.d | 34 +++++++++
> gas/testsuite/gas/riscv/dis-addr-overflow.s | 70 +++++++++++++++++++
> gas/testsuite/gas/riscv/dis-addr-topaddr-32.d | 11 +++
> gas/testsuite/gas/riscv/dis-addr-topaddr-64.d | 11 +++
> .../gas/riscv/dis-addr-topaddr-gp-32.d | 12 ++++
> .../gas/riscv/dis-addr-topaddr-gp-64.d | 12 ++++
> gas/testsuite/gas/riscv/dis-addr-topaddr-gp.s | 15 ++++
> gas/testsuite/gas/riscv/dis-addr-topaddr.s | 10 +++
> gas/testsuite/gas/riscv/lla32.d | 2 +-
> opcodes/riscv-dis.c | 46 +++++++-----
> 14 files changed, 300 insertions(+), 17 deletions(-)
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-addiw-a.d
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-addiw-b.d
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-addiw.s
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-overflow-32.d
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-overflow-64.d
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-overflow.s
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr-32.d
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr-64.d
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr-gp-32.d
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr-gp-64.d
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr-gp.s
> create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr.s
>
>
> base-commit: 46e59b72f21029f2a863e3828cec76a03283b49d
> --
> 2.34.1
>
prev parent reply other threads:[~2022-09-02 6:18 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-27 0:10 Tsukasa OI
2022-08-27 0:10 ` [PATCH v8 1/7] RISC-V: Add address printer tests with ADDIW Tsukasa OI
2022-08-27 0:10 ` [PATCH v8 2/7] RISC-V: Fix JALR target address computation Tsukasa OI
2022-08-27 0:11 ` [PATCH v8 3/7] RISC-V: Fix RV32 disassembler " Tsukasa OI
2022-08-27 0:11 ` [PATCH v8 4/7] RISC-V: Print highest address on the disassembler Tsukasa OI
2022-08-27 0:11 ` [PATCH v8 5/7] RISC-V: Print top GP-relative addresses " Tsukasa OI
2022-08-27 0:11 ` [PATCH v8 6/7] RISC-V: Clarify that `wide' is only used for ADDIW Tsukasa OI
2022-08-27 0:11 ` [PATCH v8 7/7] RISC-V: Make `is_addiw' parameter bool Tsukasa OI
2022-08-27 0:22 ` [PATCH v8 0/7] RISC-V: Fix address printer on the disassembler Tsukasa OI
2022-08-27 0:22 ` [PATCH v8 1/7] RISC-V: Add address printer tests with ADDIW Tsukasa OI
2022-08-27 0:22 ` [PATCH v8 2/7] RISC-V: Fix JALR target address computation Tsukasa OI
2022-08-27 0:22 ` [PATCH v8 3/7] RISC-V: Fix RV32 disassembler " Tsukasa OI
2022-08-27 0:22 ` [PATCH v8 4/7] RISC-V: Print highest address on the disassembler Tsukasa OI
2022-08-27 0:22 ` [PATCH v8 5/7] RISC-V: Print top GP-relative addresses " Tsukasa OI
2022-08-27 0:22 ` [PATCH v8 6/7] RISC-V: Clarify that `wide' is only used for ADDIW Tsukasa OI
2022-08-27 0:22 ` [PATCH v8 7/7] RISC-V: Make `is_addiw' parameter bool Tsukasa OI
2022-08-27 0:28 ` [PATCH v8 0/7] RISC-V: Fix address printer on the disassembler Tsukasa OI
2022-09-02 6:18 ` Nelson Chu [this message]
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='CAPpQWtA+9AcmymFbYFFz8oWsd3AdXtNHwu8GUM0HNg0U=7qhog@mail.gmail.com' \
--to=nelson@rivosinc.com \
--cc=andrew@sifive.com \
--cc=binutils@sourceware.org \
--cc=hpa@zytor.com \
--cc=jim.wilson.gcc@gmail.com \
--cc=palmer@dabbelt.com \
--cc=research_trasio@irq.a4lg.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).