From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x34.google.com (mail-oa1-x34.google.com [IPv6:2001:4860:4864:20::34]) by sourceware.org (Postfix) with ESMTPS id 991CA3858C54 for ; Fri, 2 Sep 2022 06:18:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 991CA3858C54 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-x34.google.com with SMTP id 586e51a60fabf-11e9a7135easo2525299fac.6 for ; Thu, 01 Sep 2022 23:18:37 -0700 (PDT) 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; bh=aHQU1Yyn9x2JOmFFLSv8D+bfqO6Cbh0DmiFldIgDcVA=; b=eQ44zqWT/tBMkDsmxxS1HBUBM05Z2mkzkpi7dfkBQXWyaWJeXT/MmNrGF5xashGXTe Accjb9XfJFnnOCknonY9mB4X5zlpt/2ek4J+0ta45euwVjlzUuxiv0nIWq53gZmZ7u+i KHVM9Sh5XwQ1N07FqZDhUV/9qbZR1g4TrY/MY4SR2mbZF4fPjYKMIbWKEPkKLsXYznPA 8QCqAwWuiB/vo2iITK9sb7TZKBBhuSljMxQYQvZsXlr/8YZ/y/m6WWE3Ydb33evAZU/k Fkd68CxcK0glCgUuHmlwGNNw17/wz9MzkQ6/qc2WFJ0vHyX7J47G2S/4f4KteCfTmUIs w2sw== 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; bh=aHQU1Yyn9x2JOmFFLSv8D+bfqO6Cbh0DmiFldIgDcVA=; b=LtIgDcfljEmrn22o8Lb/nW7QVNEA5AzaS84XAbocGO4ok2igQZ2HuzhdJ2A3VpsLBX jbpb3MKEsj6CmvGL1iHO67t42u/ixd21OQM2EkMlRqkOiqHQXQfs//OdIz9/eQv7gWX3 0dY7sBmQ18mdnbq+hMYPomCKLBPF1OC+zmxPpGRGc9xJddj7SNb7zOso3Eo9Brxov5Ay sGCXSLL2gqGFiGThzLdw+x/88RKJa+YoSdzdNZciBSckCKK8xFCS/B9A2XZj9hFDg667 /UDP7xP5ejZ93jU5oqGzATU18i+36JBLWG4xckeW773uqy8q3vrG7DXTaEST7pq5cfTn zecg== X-Gm-Message-State: ACgBeo2QNOkpk9FPakNpjWd0UgsA1g6sPf0Y593Fk/akgVqDHTXYfBnh eMYRe3QTHeGg//New7XRYJ0QMyTullelJk8r9ugTJw== X-Google-Smtp-Source: AA6agR6ltFcWe2P43/4KrDgNL+Cw1Q4St9hLMnO4JloKaKiNdCqTj1e01ptA+EKG8f9Devg6MJyKLr+3z4Xj6Hm2W4E= X-Received: by 2002:a05:6808:1884:b0:344:cc03:c64d with SMTP id bi4-20020a056808188400b00344cc03c64dmr1180555oib.107.1662099516906; Thu, 01 Sep 2022 23:18:36 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Nelson Chu Date: Fri, 2 Sep 2022 14:18:25 +0800 Message-ID: Subject: Re: [PATCH v8 0/7] RISC-V: Fix address printer on the disassembler To: Tsukasa OI Cc: "H . Peter Anvin" , Palmer Dabbelt , Andrew Waterman , Jim Wilson , binutils@sourceware.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,RCVD_IN_DNSWL_NONE,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: Thanks, committed patches 1-5 for pr29342. Nelson On Sat, Aug 27, 2022 at 8:11 AM 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. > > > > 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 > -- > 2.34.1 >