public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Tsukasa OI <research_trasio@irq.a4lg.com>
To: Nelson Chu <nelson@rivosinc.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics
Date: Sat, 1 Oct 2022 17:25:57 +0900	[thread overview]
Message-ID: <349dc99b-2e92-d1e7-0b9e-18c26f923d02@irq.a4lg.com> (raw)
In-Reply-To: <CAPpQWtBhhgGJXbNBuhuY9UuvP=_ByaUtS2BAQqu5yn9C3vGcyw@mail.gmail.com>

On 2022/10/01 16:21, Nelson Chu wrote:
> On Sat, Oct 1, 2022 at 1:27 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>>
>> Hi all RISC-V folks,
>>
>> GitHub tracker:
>> <https://github.com/a4lg/binutils-gdb/wiki/riscv_insn_ext_variants>
>>
>> While investigating possible implementation of both 'Z[fdq]inx' and 'P'
>> implementations, I found something common (not just register pairs).
>>
>> Here, this patch implements what I call "extension variants".
>> It was formerly a part of 'Zfinx' fixes (formerly the flag was named
>> "INSN_F_OR_X").
>>
>> If there is a instruction with multiple variants with different requirements
>> and the assembler fails to parse all variants, there is a case that needs to
>> refer ALL variants to generate proper diagnostics.
>>
>> If an instruction with "INSN_HAS_EXT_VARS" fails on all variants, the
>> assembler now has a chance to modify the instruction class for proper
>> diagnostics.  A typical use of this feature is to select wider instruction
>> class when necessary.
>>
>>
>> [Usage: CLZ instruction ('Zbpbo')]
>>
>> To implement proposed 'Zbpbo' extension, updating instruction class for
>> CLZ instruction is not enough.  If we change CLZ instruction class from
>> "INSN_CLASS_ZBB" to "INSN_CLASS_ZBB_OR_ZBPBO", we will **mistakenly**
>> support that instruction on RV64_Zbpbo because CLZ on 'Zbpbo' is RV32-only.
>>
>> Possible use with "INSN_HAS_EXT_VARS" is to define two variants of CLZ
>> instruction with that flag:
>>
>> 1.  'Zbb' (RV32/RV64)
>> 2.  'Zbpbo' (RV32)
> 
> Will we write an extra entry for clz can resolve the problem?
> 
> {"clz",        32, INSN_CLASS_ZBPBO, ...
> {"clz",        0, INSN_CLASS_ZBB, ...
> 
> Nelson

I initially thought so but it turned out that it cannot.  I learned it
when I splitted D/Zdinx and Q/Zqinx.

If an instruction entry is not matched because an extension is missing,
information to store missing extension is stored before trying the next
variant.  The point here is, this information is _overwritten_ when not
matched and only the last one is used to generate the error message when
all variants are not matched.

So (on environment with -march=rv32i with no 'Zbb' or 'Zbpbo'), just
placing two entries like that results in the message saying 'Zbb' is
required ('Zbpbo' is going to be ignored).

That's why something has to change (and I think that this is the one of
the simplest solution).

Thanks,
Tsukasa

> 
>> If both fails, we can widen the instruction class from "INSN_CLASS_ZBPBO"
>> to "INSN_CLASS_ZBB_OR_ZBPBO" if XLEN is 32.
>> After doing that, we will get following diagnostics:
>>
>> -   On RV32: 'Zbb' or 'Zbpbo' is required
>> -   On RV64: 'Zbb' is required
>>
>>
>> [Usage: 'D'/'Zdinx' or 'Q'/'Zqinx']
>>
>> Due to use of register pairs, we need to split 'D'/'Q' and 'Zdinx'/'Zqinx'
>> variants.  For instance, if parsing "fmin.d" fails on both 'D' and 'Zdinx'
>> variants, we have to require 'D' or 'Zdinx', not just 'Zdinx', the last
>> "fmin.d" variant in riscv_opcodes.
>>
>> 1.  'D'
>> 2.  'Zdinx'
>>
>> If both fails, we can widen the instruction class from "INSN_CLASS_ZDINX" to
>> "INSN_CLASS_D_OR_ZDINX".
>> After doing that, we will get diagnostics saying 'D' or 'Zdinx' is required.
>>
>>
>>
>> Thanks,
>> Tsukasa
>>
>>
>>
>>
>> Tsukasa OI (1):
>>   RISC-V: Implement extension variants
>>
>>  gas/config/tc-riscv.c  | 27 +++++++++++++++++++++++++--
>>  include/opcode/riscv.h |  5 +++++
>>  2 files changed, 30 insertions(+), 2 deletions(-)
>>
>>
>> base-commit: b4477c7f666bdeb7f8e998633c7b0cb62310b9ef
>> --
>> 2.34.1
>>
> 

  reply	other threads:[~2022-10-01  8:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-01  5:27 Tsukasa OI
2022-10-01  5:27 ` [RFC PATCH 1/1] RISC-V: Implement extension variants Tsukasa OI
2022-10-01  7:21 ` [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics Nelson Chu
2022-10-01  8:25   ` Tsukasa OI [this message]
2022-10-06 11:12     ` Nelson Chu
2022-10-06 12:08       ` Tsukasa OI

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=349dc99b-2e92-d1e7-0b9e-18c26f923d02@irq.a4lg.com \
    --to=research_trasio@irq.a4lg.com \
    --cc=binutils@sourceware.org \
    --cc=nelson@rivosinc.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).