From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x44.google.com (mail-oa1-x44.google.com [IPv6:2001:4860:4864:20::44]) by sourceware.org (Postfix) with ESMTPS id E3345385417F for ; Wed, 24 Aug 2022 11:23:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E3345385417F 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-x44.google.com with SMTP id 586e51a60fabf-11d2dcc31dbso12091577fac.7 for ; Wed, 24 Aug 2022 04:23:10 -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; bh=FQPVfgmr/GnRNi5AUSQKn4Cb0t2frBaYqBZQffOySdE=; b=F6W2cGFCWOtStjzLTwLXuPv460Uk5nANHli/639r/bNtQf+f/0Rk49R8BQ6WirKP4X Kez+81BDLqYYo5WYtDLJVwOtVFedgJw8VWNd5KJIO0ytwAoEjh9qCK7RXPvPc3BvaUh1 BZU6r6W6FjSlMpVSPziEU3gFa6t7JhJouQKxCUxMe4GMIabQXOXGUU/104xgrzeLVnwb zk0EmGQIsLIowjGBHTI1ob7rSAznmnYEpQw6ZINH/MHGpfckJOUcvMSW6q0DecThuNyx F5u6PBnQaOSGiApQGOANx5XseNvDFf1B6bfrByt+23ixQhNQcO2UGhDtKhY6n/WdaaRb u0oA== 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; bh=FQPVfgmr/GnRNi5AUSQKn4Cb0t2frBaYqBZQffOySdE=; b=cahz7H/MGgHEsqtTQvFLUS3+XuU3WVBOwUIne9KV4+WzzqZDbXfUvtIw3Wh4qhPCtF GrVwoOaM8go9VGTU20RL4v9JdLSWO1gWvSNJ3+z9W3Mf3KrHx85LXBor8uFJx0FBYeXj XhIsdemPIUCpF7fBC0hQL+0yH2biY7UMxJz7qUP6fkuLGhGYaHwv0ufvOvgilTSousaF MUyAm6tCBwcO83twhiTkzuTPo/5mwTC54wl6guMxbDyAegXNRiey7KG9vgtCfUxYA1kz 8C6TBwTvQ3DdwbbFne2SkRselth5ILzLiDN14lHdc0+x4wXB8+JOzS5C+X4+REndQuIZ Hvtg== X-Gm-Message-State: ACgBeo1ToUXOZUAd0ZEQJ6EtaFUo+ZNkgdCriZLNw8Q7rHLpd+3TZzVj Qgf+2zPXXYF3hOSYNBSvctZpzkUMioDFKX3nzZECpTCms8+oZ8kd48Y= X-Google-Smtp-Source: AA6agR4icCGRXLD1tFrtqHKh+fP+iHSP9dEDSee7wrwZHhr3pZNSQ0sieG02RFjuGc1JIGV0X9t5hRHZ0OIzdon84Y8= X-Received: by 2002:a05:6870:58aa:b0:118:307d:bb43 with SMTP id be42-20020a05687058aa00b00118307dbb43mr3422965oab.82.1661340190215; Wed, 24 Aug 2022 04:23:10 -0700 (PDT) MIME-Version: 1.0 References: <38e0a159b32579371835fbdddaf2578bae970076.1661304407.git.research_trasio@irq.a4lg.com> In-Reply-To: <38e0a159b32579371835fbdddaf2578bae970076.1661304407.git.research_trasio@irq.a4lg.com> From: Nelson Chu Date: Wed, 24 Aug 2022 19:22:59 +0800 Message-ID: Subject: Re: [PATCH v7 1/5] RISC-V: Print highest address on disassembler To: Tsukasa OI Cc: "H . Peter Anvin" , Palmer Dabbelt , Andrew Waterman , Jim Wilson , Nelson Chu , binutils@sourceware.org X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, HTML_MESSAGE, 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 Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Aug 2022 11:23:13 -0000 Hi Tsukasa, 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 Therefore, three separate patches looks reasonable. But I would suggest that we should add test cases for each patch as possible as we can, rather than add them together at last. That is because it is really hard to make sure the correctness of the separate patches. This patch refers to issue 3. So according to the source code, we won't show the final target address when pd->print_addr is -1, which means the address is exactly -1 or it is the default value. I think it is really rare to jump or refer to the symbol whose value is -1. Besides that, not showing the target address when it's value is -1 doesn't look wrong to me, so personally I would like to keep the original behavior. Unless there are other users who really want to change the behavior, or llvm are doing something different. However, the change of gp makes sense, so it looks to me. Thanks Nelson On Wed, Aug 24, 2022 at 9:28 AM Tsukasa OI via Binutils < binutils@sourceware.org> wrote: > This patch makes possible to print the highest address (0xffffffff on RV32, > 0xffffffff_ffffffff on RV64). This is particularly useful if the highest > address space is used for I/O registers and corresponding symbols > are defined. > > opcodes/ChangeLog: > > * riscv-dis.c (struct riscv_private_data): Add `to_print_addr' and > `has_gp' to enable printing the highest address. > (maybe_print_address): Utilize `to_print_addr' and `has_gp'. > (riscv_disassemble_insn): Likewise. > --- > opcodes/riscv-dis.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c > index 164fd209dbd..c6d80c3ba49 100644 > --- a/opcodes/riscv-dis.c > +++ b/opcodes/riscv-dis.c > @@ -52,6 +52,8 @@ struct riscv_private_data > bfd_vma gp; > bfd_vma print_addr; > bfd_vma hi_addr[OP_MASK_RD + 1]; > + bool to_print_addr; > + bool has_gp; > }; > > /* Used for mapping symbols. */ > @@ -177,10 +179,13 @@ maybe_print_address (struct riscv_private_data *pd, > int base_reg, int offset, > pd->print_addr = (base_reg != 0 ? pd->hi_addr[base_reg] : 0) + > offset; > pd->hi_addr[base_reg] = -1; > } > - else if (base_reg == X_GP && pd->gp != (bfd_vma)-1) > + else if (base_reg == X_GP && pd->has_gp) > pd->print_addr = pd->gp + offset; > else if (base_reg == X_TP || base_reg == 0) > pd->print_addr = offset; > + else > + return; > + pd->to_print_addr = true; > > /* Sign-extend a 32-bit value to a 64-bit value. */ > if (wide) > @@ -595,14 +600,19 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t > word, disassemble_info *info) > int i; > > pd = info->private_data = xcalloc (1, sizeof (struct > riscv_private_data)); > - pd->gp = -1; > - pd->print_addr = -1; > + pd->gp = 0; > + pd->print_addr = 0; > for (i = 0; i < (int)ARRAY_SIZE (pd->hi_addr); i++) > pd->hi_addr[i] = -1; > + pd->to_print_addr = false; > + pd->has_gp = false; > > for (i = 0; i < info->symtab_size; i++) > if (strcmp (bfd_asymbol_name (info->symtab[i]), RISCV_GP_SYMBOL) > == 0) > - pd->gp = bfd_asymbol_value (info->symtab[i]); > + { > + pd->gp = bfd_asymbol_value (info->symtab[i]); > + pd->has_gp = true; > + } > } > else > pd = info->private_data; > @@ -662,13 +672,13 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t > word, disassemble_info *info) > print_insn_args (op->args, word, memaddr, info); > > /* Try to disassemble multi-instruction addressing sequences. */ > - if (pd->print_addr != (bfd_vma)-1) > + if (pd->to_print_addr) > { > info->target = pd->print_addr; > (*info->fprintf_styled_func) > (info->stream, dis_style_comment_start, " # "); > (*info->print_address_func) (info->target, info); > - pd->print_addr = -1; > + pd->to_print_addr = false; > } > > /* Finish filling out insn_info fields. */ > -- > 2.34.1 > > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x44.google.com (mail-oa1-x44.google.com [IPv6:2001:4860:4864:20::44]) by sourceware.org (Postfix) with ESMTPS id E3345385417F for ; Wed, 24 Aug 2022 11:23:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E3345385417F 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-x44.google.com with SMTP id 586e51a60fabf-11d2dcc31dbso12091577fac.7 for ; Wed, 24 Aug 2022 04:23:10 -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; bh=FQPVfgmr/GnRNi5AUSQKn4Cb0t2frBaYqBZQffOySdE=; b=F6W2cGFCWOtStjzLTwLXuPv460Uk5nANHli/639r/bNtQf+f/0Rk49R8BQ6WirKP4X Kez+81BDLqYYo5WYtDLJVwOtVFedgJw8VWNd5KJIO0ytwAoEjh9qCK7RXPvPc3BvaUh1 BZU6r6W6FjSlMpVSPziEU3gFa6t7JhJouQKxCUxMe4GMIabQXOXGUU/104xgrzeLVnwb zk0EmGQIsLIowjGBHTI1ob7rSAznmnYEpQw6ZINH/MHGpfckJOUcvMSW6q0DecThuNyx F5u6PBnQaOSGiApQGOANx5XseNvDFf1B6bfrByt+23ixQhNQcO2UGhDtKhY6n/WdaaRb u0oA== 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; bh=FQPVfgmr/GnRNi5AUSQKn4Cb0t2frBaYqBZQffOySdE=; b=cahz7H/MGgHEsqtTQvFLUS3+XuU3WVBOwUIne9KV4+WzzqZDbXfUvtIw3Wh4qhPCtF GrVwoOaM8go9VGTU20RL4v9JdLSWO1gWvSNJ3+z9W3Mf3KrHx85LXBor8uFJx0FBYeXj XhIsdemPIUCpF7fBC0hQL+0yH2biY7UMxJz7qUP6fkuLGhGYaHwv0ufvOvgilTSousaF MUyAm6tCBwcO83twhiTkzuTPo/5mwTC54wl6guMxbDyAegXNRiey7KG9vgtCfUxYA1kz 8C6TBwTvQ3DdwbbFne2SkRselth5ILzLiDN14lHdc0+x4wXB8+JOzS5C+X4+REndQuIZ Hvtg== X-Gm-Message-State: ACgBeo1ToUXOZUAd0ZEQJ6EtaFUo+ZNkgdCriZLNw8Q7rHLpd+3TZzVj Qgf+2zPXXYF3hOSYNBSvctZpzkUMioDFKX3nzZECpTCms8+oZ8kd48Y= X-Google-Smtp-Source: AA6agR4icCGRXLD1tFrtqHKh+fP+iHSP9dEDSee7wrwZHhr3pZNSQ0sieG02RFjuGc1JIGV0X9t5hRHZ0OIzdon84Y8= X-Received: by 2002:a05:6870:58aa:b0:118:307d:bb43 with SMTP id be42-20020a05687058aa00b00118307dbb43mr3422965oab.82.1661340190215; Wed, 24 Aug 2022 04:23:10 -0700 (PDT) MIME-Version: 1.0 References: <38e0a159b32579371835fbdddaf2578bae970076.1661304407.git.research_trasio@irq.a4lg.com> In-Reply-To: <38e0a159b32579371835fbdddaf2578bae970076.1661304407.git.research_trasio@irq.a4lg.com> From: Nelson Chu Date: Wed, 24 Aug 2022 19:22:59 +0800 Message-ID: Subject: Re: [PATCH v7 1/5] RISC-V: Print highest address on disassembler To: Tsukasa OI Cc: "H . Peter Anvin" , Palmer Dabbelt , Andrew Waterman , Jim Wilson , Nelson Chu , binutils@sourceware.org Content-Type: multipart/alternative; boundary="000000000000e3abda05e6fae731" X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,HTML_MESSAGE,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: Message-ID: <20220824112259.WdzX6BHZMUPWZo5OQQl4-fEsQ9yClZezMnTuCdIoKPE@z> --000000000000e3abda05e6fae731 Content-Type: text/plain; charset="UTF-8" Hi Tsukasa, 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 Therefore, three separate patches looks reasonable. But I would suggest that we should add test cases for each patch as possible as we can, rather than add them together at last. That is because it is really hard to make sure the correctness of the separate patches. This patch refers to issue 3. So according to the source code, we won't show the final target address when pd->print_addr is -1, which means the address is exactly -1 or it is the default value. I think it is really rare to jump or refer to the symbol whose value is -1. Besides that, not showing the target address when it's value is -1 doesn't look wrong to me, so personally I would like to keep the original behavior. Unless there are other users who really want to change the behavior, or llvm are doing something different. However, the change of gp makes sense, so it looks to me. Thanks Nelson On Wed, Aug 24, 2022 at 9:28 AM Tsukasa OI via Binutils < binutils@sourceware.org> wrote: > This patch makes possible to print the highest address (0xffffffff on RV32, > 0xffffffff_ffffffff on RV64). This is particularly useful if the highest > address space is used for I/O registers and corresponding symbols > are defined. > > opcodes/ChangeLog: > > * riscv-dis.c (struct riscv_private_data): Add `to_print_addr' and > `has_gp' to enable printing the highest address. > (maybe_print_address): Utilize `to_print_addr' and `has_gp'. > (riscv_disassemble_insn): Likewise. > --- > opcodes/riscv-dis.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c > index 164fd209dbd..c6d80c3ba49 100644 > --- a/opcodes/riscv-dis.c > +++ b/opcodes/riscv-dis.c > @@ -52,6 +52,8 @@ struct riscv_private_data > bfd_vma gp; > bfd_vma print_addr; > bfd_vma hi_addr[OP_MASK_RD + 1]; > + bool to_print_addr; > + bool has_gp; > }; > > /* Used for mapping symbols. */ > @@ -177,10 +179,13 @@ maybe_print_address (struct riscv_private_data *pd, > int base_reg, int offset, > pd->print_addr = (base_reg != 0 ? pd->hi_addr[base_reg] : 0) + > offset; > pd->hi_addr[base_reg] = -1; > } > - else if (base_reg == X_GP && pd->gp != (bfd_vma)-1) > + else if (base_reg == X_GP && pd->has_gp) > pd->print_addr = pd->gp + offset; > else if (base_reg == X_TP || base_reg == 0) > pd->print_addr = offset; > + else > + return; > + pd->to_print_addr = true; > > /* Sign-extend a 32-bit value to a 64-bit value. */ > if (wide) > @@ -595,14 +600,19 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t > word, disassemble_info *info) > int i; > > pd = info->private_data = xcalloc (1, sizeof (struct > riscv_private_data)); > - pd->gp = -1; > - pd->print_addr = -1; > + pd->gp = 0; > + pd->print_addr = 0; > for (i = 0; i < (int)ARRAY_SIZE (pd->hi_addr); i++) > pd->hi_addr[i] = -1; > + pd->to_print_addr = false; > + pd->has_gp = false; > > for (i = 0; i < info->symtab_size; i++) > if (strcmp (bfd_asymbol_name (info->symtab[i]), RISCV_GP_SYMBOL) > == 0) > - pd->gp = bfd_asymbol_value (info->symtab[i]); > + { > + pd->gp = bfd_asymbol_value (info->symtab[i]); > + pd->has_gp = true; > + } > } > else > pd = info->private_data; > @@ -662,13 +672,13 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t > word, disassemble_info *info) > print_insn_args (op->args, word, memaddr, info); > > /* Try to disassemble multi-instruction addressing sequences. */ > - if (pd->print_addr != (bfd_vma)-1) > + if (pd->to_print_addr) > { > info->target = pd->print_addr; > (*info->fprintf_styled_func) > (info->stream, dis_style_comment_start, " # "); > (*info->print_address_func) (info->target, info); > - pd->print_addr = -1; > + pd->to_print_addr = false; > } > > /* Finish filling out insn_info fields. */ > -- > 2.34.1 > > --000000000000e3abda05e6fae731--