From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <research_trasio@irq.a4lg.com>
Received: from mail-sender-0.a4lg.com (mail-sender-0.a4lg.com
 [IPv6:2401:2500:203:30b:4000:6bfe:4757:0])
 by sourceware.org (Postfix) with ESMTPS id 151B5385625A
 for <binutils@sourceware.org>; Thu, 11 Aug 2022 11:43:45 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 151B5385625A
Received: from [127.0.0.1] (localhost [127.0.0.1])
 by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id B392C300089;
 Thu, 11 Aug 2022 11:43:42 +0000 (UTC)
Message-ID: <96c38e56-6454-5eb0-c0e6-6652ae14aa71@irq.a4lg.com>
Date: Thu, 11 Aug 2022 20:43:40 +0900
Mime-Version: 1.0
Subject: Re: [RFC PATCH 3/5] RISC-V: Mapping symbols with ISA string on
 assembler
Content-Language: en-US
To: Jan Beulich <jbeulich@suse.com>
Cc: binutils@sourceware.org, Nelson Chu <nelson.chu@sifive.com>,
 Kito Cheng <kito.cheng@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>
References: <1659692183-5682-1-git-send-email-nelson.chu@sifive.com>
 <cover.1660201178.git.research_trasio@irq.a4lg.com>
 <21270977a91ae08292727829cbee7ef230896d2f.1660201178.git.research_trasio@irq.a4lg.com>
 <c1ce8d56-28ad-b138-d4bf-796f7b24445c@suse.com>
From: Tsukasa OI <research_trasio@irq.a4lg.com>
In-Reply-To: <c1ce8d56-28ad-b138-d4bf-796f7b24445c@suse.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-Spam-Status: No, score=-6.5 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
X-BeenThere: binutils@sourceware.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Binutils mailing list <binutils.sourceware.org>
List-Unsubscribe: <https://sourceware.org/mailman/options/binutils>,
 <mailto:binutils-request@sourceware.org?subject=unsubscribe>
List-Archive: <https://sourceware.org/pipermail/binutils/>
List-Post: <mailto:binutils@sourceware.org>
List-Help: <mailto:binutils-request@sourceware.org?subject=help>
List-Subscribe: <https://sourceware.org/mailman/listinfo/binutils>,
 <mailto:binutils-request@sourceware.org?subject=subscribe>
X-List-Received-Date: Thu, 11 Aug 2022 11:43:47 -0000

On 2022/08/11 16:31, Jan Beulich wrote:
> On 11.08.2022 09:00, Tsukasa OI via Binutils wrote:
>> @@ -3855,11 +3887,15 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>>    if (strcmp (name, "rvc") == 0)
>>      {
>>        riscv_update_subset (&riscv_rps_as, "+c");
>> +      updated_riscv_subsets = true;
>> +      riscv_opts.arch_is_default = false;
>>        riscv_set_rvc (true);
>>      }
>>    else if (strcmp (name, "norvc") == 0)
>>      {
>>        riscv_update_subset (&riscv_rps_as, "-c");
>> +      updated_riscv_subsets = true;
>> +      riscv_opts.arch_is_default = false;
>>        riscv_set_rvc (false);
>>      }
>>    else if (strcmp (name, "pic") == 0)
>> @@ -3880,6 +3916,8 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>>        if (ISSPACE (*name) && *name != '\0')
>>  	name++;
>>        riscv_update_subset (&riscv_rps_as, name);
>> +      updated_riscv_subsets = true;
>> +      riscv_opts.arch_is_default = false;
> 
> Seeing that all three call sites of riscv_update_subset() gain the
> same extra code - wouldn't these assignments better move into that
> function? (The function living in bfd may make this difficult, but
> it being used by gas only suggests it might better be moved over. Or
> otherwise maybe add a local helper function doing all three things?)

Adding a local helper function would be the best.

> 
> As to the resulting "suffix" to $x - is this then intended to be in
> strictly canonical form?

I'm not sure but both mine and Nelson's generate "$x${CANONICAL}".

> 
>> --- a/opcodes/riscv-dis.c
>> +++ b/opcodes/riscv-dis.c
>> @@ -750,7 +750,7 @@ riscv_get_map_state (int n,
>>      return false;
>>  
>>    name = bfd_asymbol_name(info->symtab[n]);
>> -  if (strcmp (name, "$x") == 0)
>> +  if (strncmp (name, "$x", 2) == 0)
> 
> Nit: The assembler was switched to use startswith() in similar
> cases.

I'll fix this in the next version of the patch (I'll also fix PATCH 4/5).

Thanks,
Tsukasa

> 
> Jan
>