Re: Re: [PING] [PATCH 1/3] RISC-V: Add support for Zfbfmin extension

[Nelson Chu <nelson@rivosinc.com>]

[-- Attachment #1: Type: text/plain, Size: 2652 bytes --]

On Thu, Jun 6, 2024 at 2:12 PM Xiao Zeng <zengxiao@eswincomputing.com>
wrote:

> 2024-06-06 10:26  Nelson Chu <nelson@rivosinc.com> wrote:
> >1. MATCH_FCVT_S_BF16|MASK_RM, MASK_FCVT_S_BF16, looks like you put it
> >backwards?
> Do you seem to have read it backwards? Of course, it could also be my
> problem.
> If there is any further information, please feel free to ping me at any
> time.
>
> fcvt.bf16.s(single-precision -> bf16 ) is similar to
> fcvt.h.s(single-precision -> hf16 ):
>
> -------------------------------------------------------------------------------------------------------------------------------------------
> {"fcvt.h.s",   0, INSN_CLASS_ZFHMIN_INX, "D,S",
> MATCH_FCVT_H_S|MASK_RM, MASK_FCVT_H_S|MASK_RM, match_opcode, 0 },
> {"fcvt.h.s",   0, INSN_CLASS_ZFHMIN_INX, "D,S,m",   MATCH_FCVT_H_S,
> MASK_FCVT_H_S, match_opcode, 0 },
>
> {"fcvt.bf16.s", 0, INSN_CLASS_ZFBFMIN, "D,S",   MATCH_FCVT_BF16_S|MASK_RM,
> MASK_FCVT_BF16_S|MASK_RM, match_opcode, 0 },
> {"fcvt.bf16.s", 0, INSN_CLASS_ZFBFMIN, "D,S,m", MATCH_FCVT_BF16_S,
> MASK_FCVT_BF16_S, match_opcode, 0 },
>
> -------------------------------------------------------------------------------------------------------------------------------------------
>

I am saying the fcvt.s.bf16, not fcvt.bf16.s.  I thought it should be
 {"fcvt.s.bf16", 0, INSN_CLASS_ZFBFMIN,       "D,S",
 MATCH_FCVT_S_BF16|MASK_RM, MASK_FCVT_S_BF16|MASK_RM, match_opcode, 0 },
Since the spec still reserve rm operand,
https://github.com/riscv/riscv-bfloat16/blob/main/doc/insns/fcvt_S_BF16.adoc

But in fact you are right, I just remember that half to single/double
precision, single to double precision don't need to care about the rounding
mode, so your patch is right, I am confused and forgot this at first.


> >2. The fcvt.s.bf16 with "D,S,m"?
> fcvt.s.bf16(bf16 -> single-precision) is similar to fcvt.s.h(hf16 ->
> single-precision):
>
> -------------------------------------------------------------------------------------------------------------------------------------------
> {"fcvt.s.h",      0, INSN_CLASS_ZFHMIN_INX, "D,S",     MATCH_FCVT_S_H,
> MASK_FCVT_S_H|MASK_RM,       match_opcode, 0 },
> {"fcvt.s.bf16", 0, INSN_CLASS_ZFBFMIN,       "D,S",    MATCH_FCVT_S_BF16,
> MASK_FCVT_S_BF16|MASK_RM, match_opcode, 0 },
>
> -------------------------------------------------------------------------------------------------------------------------------------------
>

Forgot what I left in the previous comment, you are right.

Please send the fixed three patches again if you have time, so I can commit
them.

Thanks
Nelson