Re: [PATCH 1/9] RISC-V: Add 'H' to canonical extension ordering

[Tsukasa OI <research_trasio@irq.a4lg.com>]

On 2022/06/02 23:40, Palmer Dabbelt wrote:
> On Thu, 02 Jun 2022 07:05:58 PDT (-0700), binutils@sourceware.org wrote:
>> This commit adds 'H' to canonical extension ordering based on current
>> consensus (not officially ratified as a new ISA specification manual
>> but discussion for software compatibility is made).
>>
>> bfd/ChangeLog
>>
>>     * elfxx-riscv.c (riscv_ext_canonical_order): Add 'H' for
>>     canonical extension ordering based on current consensus.
>> ---
>>  bfd/elfxx-riscv.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
>> index 92ad03feea0..5c2c616a760 100644
>> --- a/bfd/elfxx-riscv.c
>> +++ b/bfd/elfxx-riscv.c
>> @@ -1345,7 +1345,7 @@ riscv_recognized_prefixed_ext (const char *ext)
>>  }
>>
>>  /* Canonical order for single letter extensions.  */
>> -static const char riscv_ext_canonical_order[] = "eigmafdqlcbkjtpvn";
>> +static const char riscv_ext_canonical_order[] = "eigmafdqlcbkjtpvnh";
>>
>>  /* Array is used to compare the orders of standard extensions
>> quickly.  */
>>  static int riscv_ext_order[26] = {0};
> 
> I forget if this came up elsewhere, but it just did in Linux land. 
> Essenitally we added the hypervisor CSRs/instructions to the base ISA
> under the assumption that the lack of a letter meant they were in there,
> but now that there's a letter it's unclear what we should do.  Probably
> we should do the same thing for H that we do for the other ones in
> flight right now, though.

At least, I think support for Zh* needs this commit.
Without that, it causes serious compatibility issues related to
canonical ordering (as I outlined in the cover letter).  We should
temporally remove Zh* extension support (remove Zhinx from the list:
riscv_supported_std_z_ext) unless some measure is implemented (not
necessarily this patch but I'm sure that this is the simplest).

On 2022/06/02 23:05, Tsukasa OI wrote (PATCH 0/9):
> This makes a difference.  On a machine with GPR-FP support for 16, 32
> and 64-bit floating point numbers, Jiawei's patch allows:
>     -march=rv64i_zhinx_zdinx
> but my patch allows:
>     -march=rv64i_zdinx_zhinx


For 'H', we should probably consider prototyping Binutils + GCC
implementation for new extension prefix handling (making 'H' a
single-letter extension) before new ISA Manual is released (considering
Dr.Waterman's replies, it seems there are some _private_ ISA Manual
updates [at least, not on public GitHub repo] underway).

I'll try about single-letter 'H'.

Thanks,
Tsukasa