Re: [PATCH v9 03/45] target/arm: Use FloatParts64 in f16_dotadd
On 6/9/26 08:23, Peter Maydell wrote: On Wed, 3 Jun 2026 at 04:29, Richard Henderson wrote: Use softfloat-parts.h so that we can more naturally perform the required operations witha single rounding step. This happens to also simplify the NaN detection step. Signed-off-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <[email protected]> --- diff --git a/target/arm/tcg/sme_helper.c b/target/arm/tcg/sme_helper.c index 376fbd48d4..7ef6f5d71b 100644 --- a/target/arm/tcg/sme_helper.c +++ b/target/arm/tcg/sme_helper.c @@ -27,6 +27,7 @@ #include "accel/tcg/helper-retaddr.h" #include "qemu/int128.h" #include "fpu/softfloat.h" +#include "fpu/softfloat-parts.h" #include "vec_internal.h" #include "sve_ldst_internal.h" } else { -float64 e1r = float16_to_float64(h1r, true, s_f16); -float64 e1c = float16_to_float64(h1c, true, s_f16); -float64 e2r = float16_to_float64(h2r, true, s_f16); -float64 e2c = float16_to_float64(h2c, true, s_f16); -float64 t64; - /* * The ARM pseudocode function FPDot performs both multiplies - * and the add with a single rounding operation. Emulate this - * by performing the first multiply in round-to-odd, then doing - * the second multiply as fused multiply-add, and rounding to - * float32 all in one step. + * and the add with a single rounding operation. */ -t64 = float64_mul(e1r, e2r, s_odd); -t64 = float64r32_muladd(e1c, e2c, t64, 0, s_std); +FloatParts64 tmp = parts64_mul(&p1r, &p2r, s_std); +tmp = parts64_muladd(&p1c, &p2c, &tmp, 0, s_std); This change results in our incorrectly reporting an input-denormal-used exception when FPCR.AH=1 and some of the halfprec inputs are denormal. (For AH=1 we are supposed to set IDC for input-denormal-used only for single and double precision inputs, not for halfprec. We achieve that by having vfp_get_fpsr_from_host() mask out the input_denormal_used flag from FPST_STD_F16.) Oh, right. I think this worked previously because we did a float_to_float conversion using the f16 status (float_to_float consumes an input denormal, which we don't report because we use the f16 status), and any f16 denormal is not a denormal in f64 and so the 64-bit ops never see an input denormal, so using s_std wasn't a problem ? That sounds right. (Also, for the operation using s_odd that is a copy of an fp_status so any exception flags set there never get copied back to the live version. Presumably we relied on it never being able to generate an exception?) I didn't think we were relying on that, only avoiding swapping rounding mode for each vector element. I think it doesn't matter whether we use s_std or s_f16 in the final round-and-pack step, because the only differences between the F16 and normal fpstatus are to do with input denormals (separate flush-to-zero control, and different denormal-consumed reporting), and that happens in the "unpack" and "do operation" steps; nothing cares in "pack". I went with s_f16 because we usually do the whole "unpack; operation; pack" sequence with a single fp status. Yep, thanks. r~
Re: [PATCH v9 03/45] target/arm: Use FloatParts64 in f16_dotadd
On Wed, 3 Jun 2026 at 04:29, Richard Henderson wrote: > > Use softfloat-parts.h so that we can more naturally > perform the required operations witha single rounding step. > This happens to also simplify the NaN detection step. > > Signed-off-by: Richard Henderson > Reviewed-by: Philippe Mathieu-Daudé > Message-Id: <[email protected]> > --- > diff --git a/target/arm/tcg/sme_helper.c b/target/arm/tcg/sme_helper.c > index 376fbd48d4..7ef6f5d71b 100644 > --- a/target/arm/tcg/sme_helper.c > +++ b/target/arm/tcg/sme_helper.c > @@ -27,6 +27,7 @@ > #include "accel/tcg/helper-retaddr.h" > #include "qemu/int128.h" > #include "fpu/softfloat.h" > +#include "fpu/softfloat-parts.h" > #include "vec_internal.h" > #include "sve_ldst_internal.h" > } else { > -float64 e1r = float16_to_float64(h1r, true, s_f16); > -float64 e1c = float16_to_float64(h1c, true, s_f16); > -float64 e2r = float16_to_float64(h2r, true, s_f16); > -float64 e2c = float16_to_float64(h2c, true, s_f16); > -float64 t64; > - > /* > * The ARM pseudocode function FPDot performs both multiplies > - * and the add with a single rounding operation. Emulate this > - * by performing the first multiply in round-to-odd, then doing > - * the second multiply as fused multiply-add, and rounding to > - * float32 all in one step. > + * and the add with a single rounding operation. > */ > -t64 = float64_mul(e1r, e2r, s_odd); > -t64 = float64r32_muladd(e1c, e2c, t64, 0, s_std); > +FloatParts64 tmp = parts64_mul(&p1r, &p2r, s_std); > +tmp = parts64_muladd(&p1c, &p2c, &tmp, 0, s_std); This change results in our incorrectly reporting an input-denormal-used exception when FPCR.AH=1 and some of the halfprec inputs are denormal. (For AH=1 we are supposed to set IDC for input-denormal-used only for single and double precision inputs, not for halfprec. We achieve that by having vfp_get_fpsr_from_host() mask out the input_denormal_used flag from FPST_STD_F16.) I think this worked previously because we did a float_to_float conversion using the f16 status (float_to_float consumes an input denormal, which we don't report because we use the f16 status), and any f16 denormal is not a denormal in f64 and so the 64-bit ops never see an input denormal, so using s_std wasn't a problem ? (Also, for the operation using s_odd that is a copy of an fp_status so any exception flags set there never get copied back to the live version. Presumably we relied on it never being able to generate an exception?) I think it might be right to use s_f16 instead of s_std in these three operations: @@ -1277,10 +1277,10 @@ static float32 f16_dotadd(float32 sum, uint32_t e1, uint32_t e2, * The ARM pseudocode function FPDot performs both multiplies * and the add with a single rounding operation. */ -FloatParts64 tmp = parts64_mul(&p1r, &p2r, s_std); -tmp = parts64_muladd(&p1c, &p2c, &tmp, 0, s_std); +FloatParts64 tmp = parts64_mul(&p1r, &p2r, s_f16); +tmp = parts64_muladd(&p1c, &p2c, &tmp, 0, s_f16); -t32 = float32_round_pack_canonical(&tmp, s_std); +t32 = float32_round_pack_canonical(&tmp, s_f16); } /* The final accumulation step is not fused. */ I think it doesn't matter whether we use s_std or s_f16 in the final round-and-pack step, because the only differences between the F16 and normal fpstatus are to do with input denormals (separate flush-to-zero control, and different denormal-consumed reporting), and that happens in the "unpack" and "do operation" steps; nothing cares in "pack". I went with s_f16 because we usually do the whole "unpack; operation; pack" sequence with a single fp status. thanks -- PMM
Re: [PATCH v9 03/45] target/arm: Use FloatParts64 in f16_dotadd
On 6/9/26 03:51, Peter Maydell wrote: On Wed, 3 Jun 2026 at 04:29, Richard Henderson wrote: Use softfloat-parts.h so that we can more naturally perform the required operations witha single rounding step. This happens to also simplify the NaN detection step. Signed-off-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <[email protected]> --- target/arm/tcg/sme_helper.c | 96 - 1 file changed, 40 insertions(+), 56 deletions(-) @@ -1240,48 +1238,49 @@ static float32 f16_dotadd(float32 sum, uint32_t e1, uint32_t e2, float16 h2c = e2 >> 16; float32 t32; +FloatParts64 p1r = float16_unpack_canonical(h1r, s_f16); +FloatParts64 p1c = float16_unpack_canonical(h1c, s_f16); +FloatParts64 p2r = float16_unpack_canonical(h2r, s_f16); +FloatParts64 p2c = float16_unpack_canonical(h2c, s_f16); + +int all_mask = (float_cmask(p1r.cls) | float_cmask(p1c.cls) | +float_cmask(p1r.cls) | float_cmask(p1c.cls)); This looks like a cut-and-paste error : we don't take account of p2r.cls and p2c.cls, so if those inputs are NaNs we won't handle them correctly. This should fix it: --- a/target/arm/tcg/sme_helper.c +++ b/target/arm/tcg/sme_helper.c @@ -1244,7 +1244,7 @@ static float32 f16_dotadd(float32 sum, uint32_t e1, uint32_t e2, FloatParts64 p2c = float16_unpack_canonical(h2c, s_f16); int all_mask = (float_cmask(p1r.cls) | float_cmask(p1c.cls) | -float_cmask(p1r.cls) | float_cmask(p1c.cls)); +float_cmask(p2r.cls) | float_cmask(p2c.cls)); /* C.f. FPProcessNaNs4 */ if (unlikely(all_mask & float_cmask_anynan)) { Gah. Same error in patch 1. r~
Re: [PATCH v9 03/45] target/arm: Use FloatParts64 in f16_dotadd
On Wed, 3 Jun 2026 at 04:29, Richard Henderson wrote: > > Use softfloat-parts.h so that we can more naturally > perform the required operations witha single rounding step. > This happens to also simplify the NaN detection step. > > Signed-off-by: Richard Henderson > Reviewed-by: Philippe Mathieu-Daudé > Message-Id: <[email protected]> > --- > target/arm/tcg/sme_helper.c | 96 - > 1 file changed, 40 insertions(+), 56 deletions(-) > @@ -1240,48 +1238,49 @@ static float32 f16_dotadd(float32 sum, uint32_t e1, > uint32_t e2, > float16 h2c = e2 >> 16; > float32 t32; > > +FloatParts64 p1r = float16_unpack_canonical(h1r, s_f16); > +FloatParts64 p1c = float16_unpack_canonical(h1c, s_f16); > +FloatParts64 p2r = float16_unpack_canonical(h2r, s_f16); > +FloatParts64 p2c = float16_unpack_canonical(h2c, s_f16); > + > +int all_mask = (float_cmask(p1r.cls) | float_cmask(p1c.cls) | > +float_cmask(p1r.cls) | float_cmask(p1c.cls)); This looks like a cut-and-paste error : we don't take account of p2r.cls and p2c.cls, so if those inputs are NaNs we won't handle them correctly. This should fix it: --- a/target/arm/tcg/sme_helper.c +++ b/target/arm/tcg/sme_helper.c @@ -1244,7 +1244,7 @@ static float32 f16_dotadd(float32 sum, uint32_t e1, uint32_t e2, FloatParts64 p2c = float16_unpack_canonical(h2c, s_f16); int all_mask = (float_cmask(p1r.cls) | float_cmask(p1c.cls) | -float_cmask(p1r.cls) | float_cmask(p1c.cls)); +float_cmask(p2r.cls) | float_cmask(p2c.cls)); /* C.f. FPProcessNaNs4 */ if (unlikely(all_mask & float_cmask_anynan)) { thanks -- PMM
[PATCH v9 03/45] target/arm: Use FloatParts64 in f16_dotadd
Use softfloat-parts.h so that we can more naturally perform the required operations witha single rounding step. This happens to also simplify the NaN detection step. Signed-off-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <[email protected]> --- target/arm/tcg/sme_helper.c | 96 - 1 file changed, 40 insertions(+), 56 deletions(-) diff --git a/target/arm/tcg/sme_helper.c b/target/arm/tcg/sme_helper.c index 376fbd48d4..7ef6f5d71b 100644 --- a/target/arm/tcg/sme_helper.c +++ b/target/arm/tcg/sme_helper.c @@ -27,6 +27,7 @@ #include "accel/tcg/helper-retaddr.h" #include "qemu/int128.h" #include "fpu/softfloat.h" +#include "fpu/softfloat-parts.h" #include "vec_internal.h" #include "sve_ldst_internal.h" @@ -1221,18 +1222,15 @@ static inline uint32_t bf16mop_ah_neg_adj_pair(uint32_t pair, uint32_t pg) } static float32 f16_dotadd(float32 sum, uint32_t e1, uint32_t e2, - float_status *s_f16, float_status *s_std, - float_status *s_odd) + float_status *s_f16, float_status *s_std) { /* - * We need three different float_status for different parts of this + * We need two different float_status for different parts of this * operation: * - the input conversion of the float16 values must use the *f16-specific float_status, so that the FPCR.FZ16 control is applied * - operations on float32 including the final accumulation must use *the normal float_status, so that FPCR.FZ is applied - * - we have pre-set-up copy of s_std which is set to round-to-odd, - *for the multiply (see below) */ float16 h1r = e1 & 0x; float16 h1c = e1 >> 16; @@ -1240,48 +1238,49 @@ static float32 f16_dotadd(float32 sum, uint32_t e1, uint32_t e2, float16 h2c = e2 >> 16; float32 t32; +FloatParts64 p1r = float16_unpack_canonical(h1r, s_f16); +FloatParts64 p1c = float16_unpack_canonical(h1c, s_f16); +FloatParts64 p2r = float16_unpack_canonical(h2r, s_f16); +FloatParts64 p2c = float16_unpack_canonical(h2c, s_f16); + +int all_mask = (float_cmask(p1r.cls) | float_cmask(p1c.cls) | +float_cmask(p1r.cls) | float_cmask(p1c.cls)); + /* C.f. FPProcessNaNs4 */ -if (float16_is_any_nan(h1r) || float16_is_any_nan(h1c) || -float16_is_any_nan(h2r) || float16_is_any_nan(h2c)) { +if (unlikely(all_mask & float_cmask_anynan)) { float16 t16; -if (float16_is_signaling_nan(h1r, s_f16)) { -t16 = h1r; -} else if (float16_is_signaling_nan(h1c, s_f16)) { -t16 = h1c; -} else if (float16_is_signaling_nan(h2r, s_f16)) { -t16 = h2r; -} else if (float16_is_signaling_nan(h2c, s_f16)) { -t16 = h2c; -} else if (float16_is_any_nan(h1r)) { -t16 = h1r; -} else if (float16_is_any_nan(h1c)) { -t16 = h1c; -} else if (float16_is_any_nan(h2r)) { -t16 = h2r; +if (unlikely(all_mask & float_cmask_snan)) { +if (p1r.cls == float_class_snan) { +t16 = h1r; +} else if (p1c.cls == float_class_snan) { +t16 = h1c; +} else if (p2r.cls == float_class_snan) { +t16 = h2r; +} else { +t16 = h2c; +} } else { -t16 = h2c; +if (p1r.cls == float_class_qnan) { +t16 = h1r; +} else if (p1c.cls == float_class_qnan) { +t16 = h1c; +} else if (p2r.cls == float_class_qnan) { +t16 = h2r; +} else { +t16 = h2c; +} } t32 = float16_to_float32(t16, true, s_f16); } else { -float64 e1r = float16_to_float64(h1r, true, s_f16); -float64 e1c = float16_to_float64(h1c, true, s_f16); -float64 e2r = float16_to_float64(h2r, true, s_f16); -float64 e2c = float16_to_float64(h2c, true, s_f16); -float64 t64; - /* * The ARM pseudocode function FPDot performs both multiplies - * and the add with a single rounding operation. Emulate this - * by performing the first multiply in round-to-odd, then doing - * the second multiply as fused multiply-add, and rounding to - * float32 all in one step. + * and the add with a single rounding operation. */ -t64 = float64_mul(e1r, e2r, s_odd); -t64 = float64r32_muladd(e1c, e2c, t64, 0, s_std); +FloatParts64 tmp = parts64_mul(&p1r, &p2r, s_std); +tmp = parts64_muladd(&p1c, &p2c, &tmp, 0, s_std); -/* This conversion is exact, because we've already rounded. */ -t32 = float64_to_float32(t64, s_std); +t32 = float32_round_pack_canonical(&tmp, s_std); } /* The
