Re: [PATCH qemu] fix for SME FMOPA instructions

Re: [PATCH qemu] fix for SME FMOPA instructions

[Richard Henderson]

On 7/23/24 23:21, ~danikhan632 wrote:
> From: Daniyal Khan <danikhan632@gmail.com>
> 
> ---
>   target/arm/tcg/sme_helper.c    | 122 +++++++++++++++++++++------------
>   target/arm/tcg/translate-sme.c |  37 +++++++++-
>   2 files changed, 115 insertions(+), 44 deletions(-)
> 
> diff --git a/target/arm/tcg/sme_helper.c b/target/arm/tcg/sme_helper.c
> index e2e05750399..e95045dfecc 100644
> --- a/target/arm/tcg/sme_helper.c
> +++ b/target/arm/tcg/sme_helper.c
> @@ -893,15 +893,16 @@ void HELPER(sme_fmopa_s)(void *vza, void *vzn, void *vzm, void *vpn,
>       intptr_t row, col, oprsz = simd_maxsz(desc);
>       uint32_t neg = simd_data(desc) << 31;
>       uint16_t *pn = vpn, *pm = vpm;
> -    float_status fpst;
> +    float_status *fpst = vst;
> +    float_status local_fpst;
>   
>       /*
>        * Make a copy of float_status because this operation does not
>        * update the cumulative fp exception status.  It also produces
>        * default nans.
>        */
> -    fpst = *(float_status *)vst;
> -    set_default_nan_mode(true, &fpst);
> +    local_fpst = *fpst;
> +    set_default_nan_mode(true, &local_fpst);
>   
>       for (row = 0; row < oprsz; ) {
>           uint16_t pa = pn[H2(row >> 4)];
> @@ -916,8 +917,8 @@ void HELPER(sme_fmopa_s)(void *vza, void *vzn, void *vzm, void *vpn,
>                           if (pb & 1) {
>                               uint32_t *a = vza_row + H1_4(col);
>                               uint32_t *m = vzm + H1_4(col);
> -                            *a = float32_muladd(n, *m, *a, 0, vst);
> -                        }
> +                            *a = float32_muladd(n, *m, *a, 0, &local_fpst);

This is now fixed upstream with 31d93fedf41c24b0badb38cd9317590d1ef74e37.

> +static float64 float16_to_float64_denormal(uint16_t a, float_status *status)
> +{
> +    if ((a & 0x7c00) == 0 && (a & 0x03ff) != 0) {
> +        /* This is a denormal number */
> +        int16_t exp = -14;
> +        uint16_t frac = a & 0x03ff;
> +        while ((frac & 0x0400) == 0) {
> +            frac <<= 1;
> +            exp--;
> +        }
> +        /* Remove the implicit bit */
> +        frac &= 0x03ff;
> +        /* Create the float64 */
> +        uint64_t f64 = ((uint64_t)(a & 0x8000) << 48) |
> +                       ((uint64_t)(exp + 1023) << 52) |
> +                       ((uint64_t)frac << 42);
> +        return *(float64 *)&f64;
> +    } else {
> +        /* For normal numbers, use the existing function*/
> +        return float16_to_float64(a, false, status);
> +    }
> +}


Why do you believe this is needed?  Conversion is handled correctly in float16_to_float64. 
  The same goes for the rest of the patch.


r~