Re: [PATCH] Fix ARM optimized fma & fmaf implementations

On 29/06/2024 21:44, Mickaël Thomas wrote:
> The vfma.f32|64 z, x, y instruction performs the operation
> z += x * y without intermediate rounding.
> 
> The register used for z is both read and written by the instruction.
> The inline assembly must therefore use the "+" constraint modifier.
> Using "=" the generated assembly gives an incorrect result.
> 
> Current (incorrect) implementation:
> 
> In archive arm-none-eabi/thumb/v8-m.main+dp/hard/newlib/libm.a:
> 
> <fma>:
>         eea0 0b01       vfma.f64        d0, d0, d1
>         4770            bx      lr
> <fmaf>:
>         eea0 0a20       vfma.f32        s0, s0, s1
>         4770            bx      lr
> 
> Correct implementation:
> 
> <fma>:
>         eea0 2b01       vfma.f64        d2, d0, d1
>         eeb0 0b42       vmov.f64        d0, d2
>         4770            bx      lr
> 
> <fmaf>:
>         eea0 1a20       vfma.f32        s2, s0, s1
>         eeb0 0a41       vmov.f32        s0, s2
>         4770            bx      lr
> ---
>  newlib/libm/machine/arm/s_fma_arm.c  | 2 +-
>  newlib/libm/machine/arm/sf_fma_arm.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/newlib/libm/machine/arm/s_fma_arm.c b/newlib/libm/machine/arm/s_fma_arm.c
> index f945419b5..6a2b403e1 100644
> --- a/newlib/libm/machine/arm/s_fma_arm.c
> +++ b/newlib/libm/machine/arm/s_fma_arm.c
> @@ -41,7 +41,7 @@
>  double
>  fma (double x, double y, double z)
>  {
> -  asm ("vfma.f64 %P0, %P1, %P2" : "=w" (z) : "w" (x), "w" (y));
> +  asm ("vfma.f64 %P0, %P1, %P2" : "+w" (z) : "w" (x), "w" (y));
>    return z;
>  }
>  
> diff --git a/newlib/libm/machine/arm/sf_fma_arm.c b/newlib/libm/machine/arm/sf_fma_arm.c
> index 4befd9017..8d1b63d99 100644
> --- a/newlib/libm/machine/arm/sf_fma_arm.c
> +++ b/newlib/libm/machine/arm/sf_fma_arm.c
> @@ -41,7 +41,7 @@
>  float
>  fmaf (float x, float y, float z)
>  {
> -  asm ("vfma.f32 %0, %1, %2" : "=t" (z) : "t" (x), "t" (y));
> +  asm ("vfma.f32 %0, %1, %2" : "+t" (z) : "t" (x), "t" (y));
>    return z;
>  }
>  

Nice catch.  Patch applied.

Thank you.

R.