diff mbox series

RISC-V/libgcc: Reduce the size of RV64 millicode by 6 bytes

Message ID alpine.LFD.2.21.2007301646470.24175@redsun52.ssa.fujisawa.hgst.com
State Accepted
Headers show
Series RISC-V/libgcc: Reduce the size of RV64 millicode by 6 bytes | expand

Commit Message

Maciej W. Rozycki July 30, 2020, 8:56 p.m. UTC
Rewrite code sequences throughout the 64-bit RISC-V `__riscv_save_*' 
routines replacing `li t1, -48', `li t1, -64', and `li t1, -80', 
instructions, which do not have a compressed encoding, respectively with 
`li t1, 3', `li t1, 4', and `li t1, 4', which do, and then adjusting the 
remaining code accordingly observing that `sub sp, sp, t1' takes the 
same amount of space as an `slli t1, t1, 4'/`add sp, sp, t1' instruction 
pair does, again due to the use of compressed encodings, saving 6 bytes 
total.

This change does increase code size by 4 bytes for RISC-V processors 
lacking the compressed instruction set, however their users couldn't 
care about the code size or they would have chosen an implementation 
that does have the compressed instructions, wouldn't they?

	libgcc/
	* config/riscv/save-restore.S [__riscv_xlen == 64] 
	(__riscv_save_10, __riscv_save_8, __riscv_save_6, __riscv_save_4)
	(__riscv_save_2): Replace negative immediates used for the final 
	stack pointer adjustment with positive ones, right-shifted by 4.
---
Hi,

 This is hopefully functionally obviously correct.  There were also no 
regressions in `riscv64-linux-gnu' lp64d multilib testing across all our 
testsuites (with QEMU run in the Linux user emulation mode) where target 
libraries, including glibc, have been built with `-Os -msave-restore', 
that is with millicode enabled.

 OK to apply?

  Maciej
---
 libgcc/config/riscv/save-restore.S |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

gcc-riscv-libgcc-save-sll.diff

Comments

Andrew Waterman July 30, 2020, 10:27 p.m. UTC | #1
IIRC, I didn't use this approach originally because I wanted to avoid
the additional dynamic instruction.  But I agree that code size is the
more important metric for users of this feature.  LGTM.


On Thu, Jul 30, 2020 at 1:56 PM Maciej W. Rozycki via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Rewrite code sequences throughout the 64-bit RISC-V `__riscv_save_*'
> routines replacing `li t1, -48', `li t1, -64', and `li t1, -80',
> instructions, which do not have a compressed encoding, respectively with
> `li t1, 3', `li t1, 4', and `li t1, 4', which do, and then adjusting the
> remaining code accordingly observing that `sub sp, sp, t1' takes the
> same amount of space as an `slli t1, t1, 4'/`add sp, sp, t1' instruction
> pair does, again due to the use of compressed encodings, saving 6 bytes
> total.
>
> This change does increase code size by 4 bytes for RISC-V processors
> lacking the compressed instruction set, however their users couldn't
> care about the code size or they would have chosen an implementation
> that does have the compressed instructions, wouldn't they?
>
>         libgcc/
>         * config/riscv/save-restore.S [__riscv_xlen == 64]
>         (__riscv_save_10, __riscv_save_8, __riscv_save_6, __riscv_save_4)
>         (__riscv_save_2): Replace negative immediates used for the final
>         stack pointer adjustment with positive ones, right-shifted by 4.
> ---
> Hi,
>
>  This is hopefully functionally obviously correct.  There were also no
> regressions in `riscv64-linux-gnu' lp64d multilib testing across all our
> testsuites (with QEMU run in the Linux user emulation mode) where target
> libraries, including glibc, have been built with `-Os -msave-restore',
> that is with millicode enabled.
>
>  OK to apply?
>
>   Maciej
> ---
>  libgcc/config/riscv/save-restore.S |   13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> gcc-riscv-libgcc-save-sll.diff
> Index: gcc/libgcc/config/riscv/save-restore.S
> ===================================================================
> --- gcc.orig/libgcc/config/riscv/save-restore.S
> +++ gcc/libgcc/config/riscv/save-restore.S
> @@ -45,7 +45,7 @@ FUNC_BEGIN (__riscv_save_10)
>    .cfi_restore 27
>    addi sp, sp, -112
>    .cfi_def_cfa_offset 112
> -  li t1, -16
> +  li t1, 1
>  .Ls10:
>    sd s10, 16(sp)
>    .cfi_offset 26, -96
> @@ -60,7 +60,7 @@ FUNC_BEGIN (__riscv_save_8)
>    .cfi_restore 27
>    addi sp, sp, -112
>    .cfi_def_cfa_offset 112
> -  li t1, -32
> +  li t1, 2
>  .Ls8:
>    sd s8, 32(sp)
>    .cfi_offset 24, -80
> @@ -77,7 +77,7 @@ FUNC_BEGIN (__riscv_save_6)
>    .cfi_restore 27
>    addi sp, sp, -112
>    .cfi_def_cfa_offset 112
> -  li t1, -48
> +  li t1, 3
>  .Ls6:
>    sd s6, 48(sp)
>    .cfi_offset 22, -64
> @@ -99,7 +99,7 @@ FUNC_BEGIN (__riscv_save_4)
>    .cfi_restore 27
>    addi sp, sp, -112
>    .cfi_def_cfa_offset 112
> -  li t1, -64
> +  li t1, 4
>  .Ls4:
>    sd s4, 64(sp)
>    .cfi_offset 20, -48
> @@ -123,7 +123,7 @@ FUNC_BEGIN (__riscv_save_2)
>    .cfi_restore 27
>    addi sp, sp, -112
>    .cfi_def_cfa_offset 112
> -  li t1, -80
> +  li t1, 5
>  .Ls2:
>    sd s2, 80(sp)
>    .cfi_offset 18, -32
> @@ -133,9 +133,10 @@ FUNC_BEGIN (__riscv_save_2)
>    .cfi_offset 8, -16
>    sd ra, 104(sp)
>    .cfi_offset 1, -8
> +  slli t1, t1, 4
>    # CFA info is not correct in next 2 instruction since t1's
>    # value is depend on how may register really save.
> -  sub sp, sp, t1
> +  add sp, sp, t1
>    jr t0
>    .cfi_endproc
>  FUNC_END (__riscv_save_12)
Maciej W. Rozycki July 31, 2020, 10:57 p.m. UTC | #2
On Thu, 30 Jul 2020, Andrew Waterman wrote:

> IIRC, I didn't use this approach originally because I wanted to avoid
> the additional dynamic instruction.  But I agree that code size is the
> more important metric for users of this feature.  LGTM.

 Applied now, thanks for your review.

  Maciej
diff mbox series

Patch

Index: gcc/libgcc/config/riscv/save-restore.S
===================================================================
--- gcc.orig/libgcc/config/riscv/save-restore.S
+++ gcc/libgcc/config/riscv/save-restore.S
@@ -45,7 +45,7 @@  FUNC_BEGIN (__riscv_save_10)
   .cfi_restore 27
   addi sp, sp, -112
   .cfi_def_cfa_offset 112
-  li t1, -16
+  li t1, 1
 .Ls10:
   sd s10, 16(sp)
   .cfi_offset 26, -96
@@ -60,7 +60,7 @@  FUNC_BEGIN (__riscv_save_8)
   .cfi_restore 27
   addi sp, sp, -112
   .cfi_def_cfa_offset 112
-  li t1, -32
+  li t1, 2
 .Ls8:
   sd s8, 32(sp)
   .cfi_offset 24, -80
@@ -77,7 +77,7 @@  FUNC_BEGIN (__riscv_save_6)
   .cfi_restore 27
   addi sp, sp, -112
   .cfi_def_cfa_offset 112
-  li t1, -48
+  li t1, 3
 .Ls6:
   sd s6, 48(sp)
   .cfi_offset 22, -64
@@ -99,7 +99,7 @@  FUNC_BEGIN (__riscv_save_4)
   .cfi_restore 27
   addi sp, sp, -112
   .cfi_def_cfa_offset 112
-  li t1, -64
+  li t1, 4
 .Ls4:
   sd s4, 64(sp)
   .cfi_offset 20, -48
@@ -123,7 +123,7 @@  FUNC_BEGIN (__riscv_save_2)
   .cfi_restore 27
   addi sp, sp, -112
   .cfi_def_cfa_offset 112
-  li t1, -80
+  li t1, 5
 .Ls2:
   sd s2, 80(sp)
   .cfi_offset 18, -32
@@ -133,9 +133,10 @@  FUNC_BEGIN (__riscv_save_2)
   .cfi_offset 8, -16
   sd ra, 104(sp)
   .cfi_offset 1, -8
+  slli t1, t1, 4
   # CFA info is not correct in next 2 instruction since t1's
   # value is depend on how may register really save.
-  sub sp, sp, t1
+  add sp, sp, t1
   jr t0
   .cfi_endproc
 FUNC_END (__riscv_save_12)