diff mbox series

arm: Fix PCS for SFmode -> SImode libcalls [PR99748]

Message ID 20210401161113.3eztd7en7eku4a32@arm.com
State New
Headers show
Series arm: Fix PCS for SFmode -> SImode libcalls [PR99748] | expand

Commit Message

Alex Coplan April 1, 2021, 4:11 p.m. UTC 
Hi all,

This patch fixes PR99748 which shows us trying to pass the argument to
__aeabi_f2iz in the VFP register s0 when the library function is
expecting to use the GPR r0. It also fixes the __aeabi_f2uiz case which
was broken in the same way.

For the testcase in the PR, here is the code we generate before the
patch (with -mfloat-abi=hard -march=armv8.1-m.main+mve -O0):

main:
    push    {r7, lr}
    sub     sp, sp, #8
    add     r7, sp, #0
    mov     r3, #1065353216
    str     r3, [r7, #4]    @ float
    vldr.32 s0, [r7, #4]
    bl      __aeabi_f2iz
    mov     r3, r0
    cmp     r3, #1
    [...]

This becomes:

main:
    push    {r7, lr}
    sub     sp, sp, #8
    add     r7, sp, #0
    mov     r3, #1065353216
    str     r3, [r7, #4]    @ float
    ldr     r0, [r7, #4]    @ float
    bl      __aeabi_f2iz
    mov     r3, r0
    cmp     r3, #1
    [...]

after the patch. We see a similar change for the same testcase with a
cast to unsigned instead of int.

Testing:
 * Bootstrapped and regtested on arm-linux-gnueabihf, no regressions.
 * Regtested an arm-eabi cross configured with --with-float=hard
   --with-arch=armv8.1-m.main+mve. This shows that the patch fixes the
   following execution failures:

   FAIL->PASS: gcc.c-torture/execute/20000605-1.c   -O0  execution test
   FAIL->PASS: gcc.c-torture/execute/conversion.c   -O0  execution test
   FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O0  execution test
   FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O1  execution test
   FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O2  execution test
   FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
   FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O3 -g  execution test
   FAIL->PASS: gcc.c-torture/execute/float-floor.c   -Os  execution test
   FAIL->PASS: gcc.c-torture/execute/gofast.c   -O0  execution test
   FAIL->PASS: gcc.dg/torture/float32-basic.c   -O0  execution test
   FAIL->PASS: gcc.dg/torture/float32-basic.c   -O1  execution test
   FAIL->PASS: gcc.dg/torture/float32-basic.c   -O2  execution test
   FAIL->PASS: gcc.dg/torture/float32-basic.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
   FAIL->PASS: gcc.dg/torture/float32-basic.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  execution test
   FAIL->PASS: gcc.dg/torture/float32-basic.c   -O3 -g  execution test
   FAIL->PASS: gcc.dg/torture/float32-basic.c   -Os  execution test

OK for trunk?

Thanks,
Alex

gcc/ChangeLog:

	* config/arm/arm.c (arm_libcall_uses_aapcs_base): Also use base
	PCS for [su]fix_optab.

Comments

Richard Earnshaw April 1, 2021, 5:35 p.m. UTC | #1 
On 01/04/2021 17:11, Alex Coplan via Gcc-patches wrote:
> Hi all,
> 
> This patch fixes PR99748 which shows us trying to pass the argument to
> __aeabi_f2iz in the VFP register s0 when the library function is
> expecting to use the GPR r0. It also fixes the __aeabi_f2uiz case which
> was broken in the same way.
> 
> For the testcase in the PR, here is the code we generate before the
> patch (with -mfloat-abi=hard -march=armv8.1-m.main+mve -O0):
> 
> main:
>      push    {r7, lr}
>      sub     sp, sp, #8
>      add     r7, sp, #0
>      mov     r3, #1065353216
>      str     r3, [r7, #4]    @ float
>      vldr.32 s0, [r7, #4]
>      bl      __aeabi_f2iz
>      mov     r3, r0
>      cmp     r3, #1
>      [...]
> 
> This becomes:
> 
> main:
>      push    {r7, lr}
>      sub     sp, sp, #8
>      add     r7, sp, #0
>      mov     r3, #1065353216
>      str     r3, [r7, #4]    @ float
>      ldr     r0, [r7, #4]    @ float
>      bl      __aeabi_f2iz
>      mov     r3, r0
>      cmp     r3, #1
>      [...]
> 
> after the patch. We see a similar change for the same testcase with a
> cast to unsigned instead of int.
> 
> Testing:
>   * Bootstrapped and regtested on arm-linux-gnueabihf, no regressions.
>   * Regtested an arm-eabi cross configured with --with-float=hard
>     --with-arch=armv8.1-m.main+mve. This shows that the patch fixes the
>     following execution failures:
> 
>     FAIL->PASS: gcc.c-torture/execute/20000605-1.c   -O0  execution test
>     FAIL->PASS: gcc.c-torture/execute/conversion.c   -O0  execution test
>     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O0  execution test
>     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O1  execution test
>     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O2  execution test
>     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
>     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O3 -g  execution test
>     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -Os  execution test
>     FAIL->PASS: gcc.c-torture/execute/gofast.c   -O0  execution test
>     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O0  execution test
>     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O1  execution test
>     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O2  execution test
>     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
>     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  execution test
>     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O3 -g  execution test
>     FAIL->PASS: gcc.dg/torture/float32-basic.c   -Os  execution test
> 
> OK for trunk?
> 
> Thanks,
> Alex
> 
> gcc/ChangeLog:
> 
> 	* config/arm/arm.c (arm_libcall_uses_aapcs_base): Also use base
> 	PCS for [su]fix_optab.
> 

OK.

As a wrong code bug we should probably be looking to backport this if 
needed (though it's likely too late now for 10.3).

R.
Alex Coplan April 23, 2021, 11:48 a.m. UTC | #2 
On 01/04/2021 18:35, Richard Earnshaw wrote:
> 
> 
> On 01/04/2021 17:11, Alex Coplan via Gcc-patches wrote:
> > Hi all,
> > 
> > This patch fixes PR99748 which shows us trying to pass the argument to
> > __aeabi_f2iz in the VFP register s0 when the library function is
> > expecting to use the GPR r0. It also fixes the __aeabi_f2uiz case which
> > was broken in the same way.
> > 
> > For the testcase in the PR, here is the code we generate before the
> > patch (with -mfloat-abi=hard -march=armv8.1-m.main+mve -O0):
> > 
> > main:
> >      push    {r7, lr}
> >      sub     sp, sp, #8
> >      add     r7, sp, #0
> >      mov     r3, #1065353216
> >      str     r3, [r7, #4]    @ float
> >      vldr.32 s0, [r7, #4]
> >      bl      __aeabi_f2iz
> >      mov     r3, r0
> >      cmp     r3, #1
> >      [...]
> > 
> > This becomes:
> > 
> > main:
> >      push    {r7, lr}
> >      sub     sp, sp, #8
> >      add     r7, sp, #0
> >      mov     r3, #1065353216
> >      str     r3, [r7, #4]    @ float
> >      ldr     r0, [r7, #4]    @ float
> >      bl      __aeabi_f2iz
> >      mov     r3, r0
> >      cmp     r3, #1
> >      [...]
> > 
> > after the patch. We see a similar change for the same testcase with a
> > cast to unsigned instead of int.
> > 
> > Testing:
> >   * Bootstrapped and regtested on arm-linux-gnueabihf, no regressions.
> >   * Regtested an arm-eabi cross configured with --with-float=hard
> >     --with-arch=armv8.1-m.main+mve. This shows that the patch fixes the
> >     following execution failures:
> > 
> >     FAIL->PASS: gcc.c-torture/execute/20000605-1.c   -O0  execution test
> >     FAIL->PASS: gcc.c-torture/execute/conversion.c   -O0  execution test
> >     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O0  execution test
> >     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O1  execution test
> >     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O2  execution test
> >     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
> >     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -O3 -g  execution test
> >     FAIL->PASS: gcc.c-torture/execute/float-floor.c   -Os  execution test
> >     FAIL->PASS: gcc.c-torture/execute/gofast.c   -O0  execution test
> >     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O0  execution test
> >     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O1  execution test
> >     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O2  execution test
> >     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
> >     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  execution test
> >     FAIL->PASS: gcc.dg/torture/float32-basic.c   -O3 -g  execution test
> >     FAIL->PASS: gcc.dg/torture/float32-basic.c   -Os  execution test
> > 
> > OK for trunk?
> > 
> > Thanks,
> > Alex
> > 
> > gcc/ChangeLog:
> > 
> > 	* config/arm/arm.c (arm_libcall_uses_aapcs_base): Also use base
> > 	PCS for [su]fix_optab.
> > 
> 
> OK.
> 
> As a wrong code bug we should probably be looking to backport this if needed
> (though it's likely too late now for 10.3).

Testing shows the patch fixes the issue on the 10 branch. Bootstrapped
on arm-linux-gnueabihf and regtested an MVE cross: no issues.

Cherry-picked as
r10-9755-g283367662c25057fd7c9c98257cca858f85b75fc.

> 
> R.

Thanks,
Alex
diff mbox series

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 518bfed1c35..8910dad8214 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -5773,6 +5773,10 @@  arm_libcall_uses_aapcs_base (const_rtx libcall)
 		   convert_optab_libfunc (sfix_optab, DImode, SFmode));
       add_libcall (libcall_htab,
 		   convert_optab_libfunc (ufix_optab, DImode, SFmode));
+      add_libcall (libcall_htab,
+		   convert_optab_libfunc (sfix_optab, SImode, SFmode));
+      add_libcall (libcall_htab,
+		   convert_optab_libfunc (ufix_optab, SImode, SFmode));
 
       /* Values from double-precision helper functions are returned in core
 	 registers if the selected core only supports single-precision