[RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics

[Tsukasa OI]

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
>>
>