On Wed, Mar 04, 2026 at 09:25:12PM +0800, Max Chou wrote:
> According to the Zvfbfa ISA spec v0.1, the following vector floating
> point instructions have different bahaviors depend on the ALTFMT and
"bahaviors" -> "behaviors".
[...]
> static bool opfvf_trans(uint32_t vd, uint32_t rs1, uint32_t vs2,
> - uint32_t data, gen_helper_opfvf *fn, DisasContext *s)
> + uint32_t data, gen_helper_opfvf *fn, DisasContext *s,
> + bool do_bf16_nanbox)
> {
> TCGv_ptr dest, src2, mask;
> TCGv_i32 desc;
> @@ -2403,7 +2461,7 @@ static bool opfvf_trans(uint32_t vd, uint32_t rs1,
> uint32_t vs2,
>
> /* NaN-box f[rs1] */
> t1 = tcg_temp_new_i64();
> - do_nanbox(s, t1, cpu_fpr[rs1]);
> + do_nanbox(s, t1, cpu_fpr[rs1], do_bf16_nanbox);
this is a bug.
All arithmetic OPFVF BFA instructions pass
`do_bf16_nanbox=false`, but they should pass `true`.
When altfmt=1 and a scalar f-register operand is
improperly NaN-boxed, the code substitutes FP16
canonical NaN (0x7e00). But 0x7e00 interpreted as
BF16 is a normal finite number (~2^125), NOT NaN.
This produces incorrect arithmetic results.
static void do_nanbox(DisasContext *s, TCGv_i64 out, TCGv_i64 in,
bool do_bf16_nanbox)
{
if (s->sew == MO_16) {
if (s->altfmt && do_bf16_nanbox) { /* real: s->altfmt && false */
gen_check_nanbox_h_bf16(out, in); /* NaN: 0x7fc0 */
} else {
gen_check_nanbox_h(out, in); /* NaN: 0x7e00 */
}
}
[...]
}
Affected callers:
> GEN_OPFVF_BFA_TRANS(vfadd_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfsub_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfrsub_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfmul_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfmacc_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfnmacc_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfmsac_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfnmsac_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfmadd_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfnmadd_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfmsub_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfnmsub_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfmin_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vfmax_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vmfeq_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vmfne_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vmflt_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vmfle_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vmfgt_vf, ..., false)
> GEN_OPFVF_BFA_TRANS(vmfge_vf, ..., false)
[...]
All of these should be `true`. The widen macros
(GEN_OPFVF_WIDEN_BFA_TRANS, GEN_OPFWF_WIDEN_BFA_TRANS)
hardcode `false` and should also be `true`.
The sign-injection (vfsgnj/vfsgnjn/vfsgnjx),
vfmerge, vfslide1up/down, vfmv.v.f, and vfmv.s.f
correctly pass `true`. The intent seems clear --
the `false` values are simply a mistake.
I suggest that since all BFA callers are supposed to
pass true, we consider removing the BF16_NANBOX parameter
entirely and having do_nanbox() directly check s->altfmt
(this is what patch 7 originally implemented before
patch 8 added the parameter).
Reviewed-by: Chao Liu <[email protected]>
Best regards,
Chao Liu
