Re: [PATCH] target/arm: Handle denormals correctly for FMOPA (widening)

[Peter Maydell]

On Tue, 30 Jul 2024 at 16:58, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The FMOPA (widening) SME instruction takes pairs of half-precision
> floating point values, widens them to single-precision, does a
> two-way dot product and accumulates the results into a
> single-precision destination.  We don't quite correctly handle the
> FPCR bits FZ and FZ16 which control flushing of denormal inputs and
> outputs.  This is because at the moment we pass a single float_status
> value to the helper function, which then uses that configuration for
> all the fp operations it does.  However, because the inputs to this
> operation are float16 and the outputs are float32 we need to use the
> fp_status_f16 for the float16 input widening but the normal fp_status
> for everything else.  Otherwise we will apply the flushing control
> FPCR.FZ16 to the 32-bit output rather than the FPCR.FZ control, and
> incorrectly flush a denormal output to zero when we should not (or
> vice-versa).
>
> Pass the CPU env to the sme_fmopa_h helper instead of an fp_status
> pointer, and have the helper pass an extra fp_status into the
> f16_dotadd() function so that we can use the right status for the
> right parts of this operation.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

I just noticed that there's an associated bug in gitlab
and that we had a go at fixing the denormal-flushing of this
insn in commit 207d30b5fdb5 recently. So I'm adding a note

 (In commit 207d30b5fdb5b we tried to fix the FZ handling but
 didn't get it right, switching from "use FPCR.FZ for everything" to
 "use FPCR.FZ16 for everything".)

and

Fixes: 207d30b5fdb5 ("target/arm: Use FPST_F16 for SME FMOPA (widening)")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2373

to the commit message for this. (I've taken it into target-arm.next.)

thanks
-- PMM