From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender.a4lg.com [153.120.152.154]) by sourceware.org (Postfix) with ESMTPS id 40A5B3834F05 for ; Sat, 27 Aug 2022 00:28:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 40A5B3834F05 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=irq.a4lg.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=irq.a4lg.com Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id 994BD300089; Sat, 27 Aug 2022 00:28:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=irq.a4lg.com; s=2017s01; t=1661560095; bh=Rur8AFT3I7RT67z+VW/syCAjJ3ZpyGa9Z3UAUYQt1Tk=; h=Message-ID:Date:Mime-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=XKuNMoTZrS9q3Z7ymOhqU+Y4Edp/rtZaJP/7i400dyaNgahXkfftEaaOxUZTAmgmA QPfTLgoNmP9voHatDH/2ru7Ky+jeEFcaUNLQalTJ1UQskOt5YcSSfZeY3daJ5I+O0W t5otJyK+k0aDfOPQBomjBeVcUE9vgtP3snhMZPqo= Message-ID: <0d7fb2cc-077c-8c62-a923-cee7bfdc3065@irq.a4lg.com> Date: Sat, 27 Aug 2022 09:28:14 +0900 Mime-Version: 1.0 Subject: Re: [PATCH v8 0/7] RISC-V: Fix address printer on the disassembler To: Nelson Chu Cc: binutils@sourceware.org References: Content-Language: en-US From: Tsukasa OI In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: Gahhh... this time, I now have wrong subject lines. Nothing goes right if I'm in a hurry. Please consider the base e-mail (with [Changes: v8 -> v9]) as PATCH v9. Thanks, Tsukasa On 2022/08/27 9:22, Tsukasa OI wrote: > Hello, > > Tracker on GitHub: > > > Original Issue as reported by H. Peter Anvin: > > > Previous Information on v1...v7: > > See this e-mail for background. > > > [Changes: v8 -> v9] > > Sorry! I forgot to remove redundant ChangeLog lines. > > > > 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. > > 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: > new: > # gas/testsuite/gas/riscv/dis-addr-topaddr-gp-32.d > old: > 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