Re: Re: Re: Re: Re: 回复： Re: RISCV: Support Zilsd and Zcmlsd extensions.

[Kito Cheng]

Hi Donya:

Juse one minor comment:

I saw there is a macro version of ld and sd with the symbol, but I
don't see the test case for that, could you add one?

I guess the simplest way is to use
binutils/gas/testsuite/gas/riscv/l-s-macro.s as the test case ?

> @@ -621,10 +662,18 @@ const struct riscv_opcode riscv_opcodes[] =
> {"ld",         64, INSN_CLASS_C, "Ct,Cl(Cs)", MATCH_C_LD, MASK_C_LD, match_opcode, INSN_ALIAS|INSN_DREF|INSN_8_BYTE },
> {"ld",         64, INSN_CLASS_I, "d,o(s)",    MATCH_LD, MASK_LD, match_opcode, INSN_DREF|INSN_8_BYTE },
> {"ld",         64, INSN_CLASS_I, "d,A",       0, (int) M_Lx, match_rd_nonzero, INSN_MACRO },
> +{"ld",         32, INSN_CLASS_ZCLSD, "d,Cn(Cc)",  MATCH_C_LDSP, MASK_C_LDSP, match_rd_even_nonzero, INSN_ALIAS|INSN_DREF|INSN_8_BYTE },
> +{"ld",         32, INSN_CLASS_ZCLSD, "Ct,Cl(Cs)", MATCH_C_LD, MASK_C_LD, match_crs2s_even, INSN_ALIAS|INSN_DREF|INSN_8_BYTE },
> +{"ld",         32, INSN_CLASS_ZILSD, "d,o(s)",    MATCH_LD, MASK_LD, match_rd_even, INSN_DREF|INSN_8_BYTE },
> +{"ld",         32, INSN_CLASS_ZILSD, "d,A",       0, (int) M_Lx, match_rd_even_nonzero, INSN_MACRO },

^^^^ This
e.g.
ld a0, foo

> {"sd",         64, INSN_CLASS_C, "CV,CN(Cc)", MATCH_C_SDSP, MASK_C_SDSP, match_opcode, INSN_ALIAS|INSN_DREF|INSN_8_BYTE },
> {"sd",         64, INSN_CLASS_C, "Ct,Cl(Cs)", MATCH_C_SD, MASK_C_SD, match_opcode, INSN_ALIAS|INSN_DREF|INSN_8_BYTE },
> {"sd",         64, INSN_CLASS_I, "t,q(s)",    MATCH_SD, MASK_SD, match_opcode, INSN_DREF|INSN_8_BYTE },
> {"sd",         64, INSN_CLASS_I, "t,A,s",     0, (int) M_Sx_FSx, match_rs1_nonzero, INSN_MACRO },
> +{"sd",         32, INSN_CLASS_ZCLSD, "CV,CN(Cc)", MATCH_C_SDSP, MASK_C_SDSP, match_crs2_even, INSN_ALIAS|INSN_DREF|INSN_8_BYTE },
> +{"sd",         32, INSN_CLASS_ZCLSD, "Ct,Cl(Cs)", MATCH_C_SD, MASK_C_SD, match_crs2s_even, INSN_ALIAS|INSN_DREF|INSN_8_BYTE },
> +{"sd",         32, INSN_CLASS_ZILSD, "t,q(s)",    MATCH_SD, MASK_SD, match_rs2_even, INSN_DREF|INSN_8_BYTE },
> +{"sd",         32, INSN_CLASS_ZILSD, "t,A,s",     0, (int) M_Sx_FSx, match_rs1_nonzero_rs2_even, INSN_MACRO },

^^^^ and this

e.g.
sd      a0, foo, t0

> {"sext.w",     64, INSN_CLASS_C, "d,CU",      MATCH_C_ADDIW, MASK_C_ADDIW|MASK_RVC_IMM, match_rd_nonzero, INSN_ALIAS },
> {"sext.w",     64, INSN_CLASS_I, "d,s",       MATCH_ADDIW, MASK_ADDIW|MASK_IMM, match_opcode, INSN_ALIAS },
> {"addiw",      64, INSN_CLASS_C, "d,CU,Co",   MATCH_C_ADDIW, MASK_C_ADDIW, match_rd_nonzero, INSN_ALIAS },

On Mon, Mar 31, 2025 at 4:54 PM 孙东亚 <sundongya@nucleisys.com> wrote:
>
> Thanks,
>
> Thank you for your reminder. After discussion, we have adjusted our co-author list, removed Wienskoski <edmar.wienskoski@nxp.com> from the co-author names, and updated the patch.
>
> Ref: https://github.com/riscv/riscv-zilsd/blob/main/zilsd.adoc
>
> bfd/ChangeLog:
>
>         * elfxx-riscv.c (riscv_parse_check_conflicts):"zclsd" conflict with the “c+f'/ zcf”extension.
>         (riscv_multi_subset_supports):Handle zilsd and zclsd.
>         (riscv_multi_subset_supports_ext):Handle zilsd and zclsd.
>
> gas/ChangeLog:
>
>         * testsuite/gas/riscv/zilsd-zclsd-fail.d: New test.
>         * testsuite/gas/riscv/zilsd-zclsd-fail.l: New test.
>         * testsuite/gas/riscv/zilsd-zclsd-fail.s: New test.
>         * testsuite/gas/riscv/zilsd-zclsd.d: New test.
>         * testsuite/gas/riscv/zilsd-zclsd.s: New test.
>
> include/ChangeLog:
>
>         * opcode/riscv.h (enum riscv_insn_class):Add INSN_CLASS_ZILSD and INSN_CLASS_ZCLSD.
>
> opcodes/ChangeLog:
>
>         * riscv-opc.c (MASK_CRS2S):Define the crs2s register operand mask.
>         (match_rd_even):Constrain the rd register operand.
>         (match_rs2_even):Constrain the rs2 register operand.
>         (match_rd_even_nonzero):Constrain the rd register operand.
>         (match_rs1_nonzero_rs2_even):Constrain the rs1 and rs2 register operand.
>         (match_crs2s_even):Constrain the crs2s register operand.
>         (match_crs2_even):Constrain the crs2 register operand.
>
> Signed-off-by: dysun <sundongya@nucleisys.com>
> Co-developed-by: LIU Xu <liuxu@nucleisys.com>
> Co-developed-by: ZHAO Fujin <zhaofujin@nucleisys.com>
> ---
>  bfd/elfxx-riscv.c                          | 20 ++++++++
>  gas/testsuite/gas/riscv/zilsd-zclsd-fail.d |  3 ++
>  gas/testsuite/gas/riscv/zilsd-zclsd-fail.l | 15 ++++++
>  gas/testsuite/gas/riscv/zilsd-zclsd-fail.s | 15 ++++++
>  gas/testsuite/gas/riscv/zilsd-zclsd.d      | 29 ++++++++++++
>  gas/testsuite/gas/riscv/zilsd-zclsd.s      | 20 ++++++++
>  include/opcode/riscv.h                     |  2 +
>  opcodes/riscv-opc.c                        | 54 ++++++++++++++++++++++
>  8 files changed, 158 insertions(+)
>  create mode 100644 gas/testsuite/gas/riscv/zilsd-zclsd-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/zilsd-zclsd-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/zilsd-zclsd-fail.s
>  create mode 100644 gas/testsuite/gas/riscv/zilsd-zclsd.d
>  create mode 100644 gas/testsuite/gas/riscv/zilsd-zclsd.s
>
> Nelson Chu<nelson@rivosinc.com> 在 2025年3月27日 周四 12:27 写道：
>
>
> On Wed, Mar 26, 2025 at 3:16 PM 孙东亚 <sundongya@nucleisys.com> wrote:
>>
>>
>> Thanks for your reply.
>>
>> Thanks for your advice, we've adjusted the test case, keeping only the even operands checked in the fail test cases. Also, we've merged the test cases for zilsd and zclsd.
>> About co-author, we initially found the open-source repository at https://github.com/nxp-auto-tools/binutils_zilsd/tree/zilsd for zilsd-related extension support. We submitted our implementation there. After Edmar Wienskoski <edmar.wienskoski@nxp.com> reviewed it, our submission was merged into their repository. Later, we pushed our patch upstream. So Edmar Wienskoski is our collaborator.
>> Besides, our team has three people in total. The other two collaborators are LIU Xu <liuxu@nucleisys.com> and ZHAO Fujin <zhaofujin@nucleisys.com>.
>> ...
>>
>> Signed-off-by: dysun <sundongya@nucleisys.com>
>> Co-developed-by: LIU Xu <liuxu@nucleisys.com>
>> Co-developed-by: ZHAO Fujin <zhaofujin@nucleisys.com>
>> Co-developed-by: Edmar Wienskoski <edmar.wienskoski@NXP.com>
>
>
> Okay, so the problem here is - I find that NXP company only has the copyright of gcc, rather than binutils and gdb, and I also cannot find the personal copyright of Edmar Wienskoski.  So...  Just feel free to correct me if I missed it and wrong, or is Edmar also willing to have his sign-off-by for this patch?  I am sorry that this might seem too harsh to you, but we've made the similar copyright mistakes before, so now we're very careful.
>
> Thanks a lot
> Nelson
>