[PATCH] RISC-V: decouple stack allocation for rv32e w/o save-restore.

[PATCH] RISC-V: decouple stack allocation for rv32e w/o save-restore.

[Fei Gao]

Currently in rv32e, stack allocation for GPR callee-saved registers is
always 12 bytes w/o save-restore. Actually, for the case without save-restore,
less stack memory can be reserved. This patch decouples stack allocation for
rv32e w/o save-restore and makes riscv_compute_frame_info more readable.

output of testcase rv32e_stack.c
before patch:
	addi	sp,sp,-16
	sw	ra,12(sp)
	call	getInt
	sw	a0,0(sp)
	lw	a0,0(sp)
	call	PrintInts
	lw	a5,0(sp)
	mv	a0,a5
	lw	ra,12(sp)
	addi	sp,sp,16
	jr	ra

after patch:
	addi	sp,sp,-8
	sw	ra,4(sp)
	call	getInt
	sw	a0,0(sp)
	lw	a0,0(sp)
	call	PrintInts
	lw	a5,0(sp)
	mv	a0,a5
	lw	ra,4(sp)
	addi	sp,sp,8
	jr	ra

gcc/ChangeLog:

        * config/riscv/riscv.cc (riscv_forbid_save_libcall): helper function for riscv_use_save_libcall.
        (riscv_use_save_libcall): call riscv_forbid_save_libcall.
        (riscv_compute_frame_info): restructure to decouple stack allocation for rv32e w/o save-restore.

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/rv32e_stack.c: New test.
---
 gcc/config/riscv/riscv.cc                    | 57 ++++++++++++--------
 gcc/testsuite/gcc.target/riscv/rv32e_stack.c | 14 +++++
 2 files changed, 49 insertions(+), 22 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rv32e_stack.c

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 5d2550871c7..6ccdfe96fe7 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -4772,12 +4772,26 @@ riscv_save_reg_p (unsigned int regno)
   return false;
 }
 
+/* Determine whether to disable GPR save/restore routines.  */
+static bool
+riscv_forbid_save_libcall (void)
+{
+  if (!TARGET_SAVE_RESTORE
+      || crtl->calls_eh_return
+      || frame_pointer_needed
+      || cfun->machine->interrupt_handler_p
+      || cfun->machine->varargs_size != 0
+      || crtl->args.pretend_args_size != 0)
+    return true;
+
+  return false;
+}
+
 /* Determine whether to call GPR save/restore routines.  */
 static bool
 riscv_use_save_libcall (const struct riscv_frame_info *frame)
 {
-  if (!TARGET_SAVE_RESTORE || crtl->calls_eh_return || frame_pointer_needed
-      || cfun->machine->interrupt_handler_p)
+  if (riscv_forbid_save_libcall ())
     return false;
 
   return frame->save_libcall_adjustment != 0;
@@ -4857,7 +4871,7 @@ riscv_compute_frame_info (void)
   struct riscv_frame_info *frame;
   poly_int64 offset;
   bool interrupt_save_prologue_temp = false;
-  unsigned int regno, i, num_x_saved = 0, num_f_saved = 0;
+  unsigned int regno, i, num_x_saved = 0, num_f_saved = 0, x_save_size = 0;
 
   frame = &cfun->machine->frame;
 
@@ -4895,24 +4909,14 @@ riscv_compute_frame_info (void)
 	    frame->fmask |= 1 << (regno - FP_REG_FIRST), num_f_saved++;
     }
 
-  /* At the bottom of the frame are any outgoing stack arguments. */
-  offset = riscv_stack_align (crtl->outgoing_args_size);
-  /* Next are local stack variables. */
-  offset += riscv_stack_align (get_frame_size ());
-  /* The virtual frame pointer points above the local variables. */
-  frame->frame_pointer_offset = offset;
-  /* Next are the callee-saved FPRs. */
-  if (frame->fmask)
-    offset += riscv_stack_align (num_f_saved * UNITS_PER_FP_REG);
-  frame->fp_sp_offset = offset - UNITS_PER_FP_REG;
-  /* Next are the callee-saved GPRs. */
   if (frame->mask)
     {
-      unsigned x_save_size = riscv_stack_align (num_x_saved * UNITS_PER_WORD);
+      x_save_size = riscv_stack_align (num_x_saved * UNITS_PER_WORD);
       unsigned num_save_restore = 1 + riscv_save_libcall_count (frame->mask);
 
       /* Only use save/restore routines if they don't alter the stack size.  */
-      if (riscv_stack_align (num_save_restore * UNITS_PER_WORD) == x_save_size)
+      if (riscv_stack_align (num_save_restore * UNITS_PER_WORD) == x_save_size
+          && !riscv_forbid_save_libcall ())
 	{
 	  /* Libcall saves/restores 3 registers at once, so we need to
 	     allocate 12 bytes for callee-saved register.  */
@@ -4921,9 +4925,21 @@ riscv_compute_frame_info (void)
 
 	  frame->save_libcall_adjustment = x_save_size;
 	}
-
-      offset += x_save_size;
     }
+
+  /* At the bottom of the frame are any outgoing stack arguments. */
+  offset = riscv_stack_align (crtl->outgoing_args_size);
+  /* Next are local stack variables. */
+  offset += riscv_stack_align (get_frame_size ());
+  /* The virtual frame pointer points above the local variables. */
+  frame->frame_pointer_offset = offset;
+  /* Next are the callee-saved FPRs. */
+  if (frame->fmask)
+    offset += riscv_stack_align (num_f_saved * UNITS_PER_FP_REG);
+  frame->fp_sp_offset = offset - UNITS_PER_FP_REG;
+  /* Next are the callee-saved GPRs. */
+  if (frame->mask)
+    offset += x_save_size;
   frame->gp_sp_offset = offset - UNITS_PER_WORD;
   /* The hard frame pointer points above the callee-saved GPRs. */
   frame->hard_frame_pointer_offset = offset;
@@ -4935,11 +4951,8 @@ riscv_compute_frame_info (void)
      padding.  */
   frame->arg_pointer_offset = offset - crtl->args.pretend_args_size;
   frame->total_size = offset;
-  /* Next points the incoming stack pointer and any incoming arguments. */
 
-  /* Only use save/restore routines when the GPRs are atop the frame.  */
-  if (known_ne (frame->hard_frame_pointer_offset, frame->total_size))
-    frame->save_libcall_adjustment = 0;
+  /* Next points the incoming stack pointer and any incoming arguments. */
 }
 
 /* Make sure that we're not trying to eliminate to the wrong hard frame
diff --git a/gcc/testsuite/gcc.target/riscv/rv32e_stack.c b/gcc/testsuite/gcc.target/riscv/rv32e_stack.c
new file mode 100644
index 00000000000..cec90ede4b5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rv32e_stack.c
@@ -0,0 +1,14 @@
+/* { dg-do compile} */
+/* { dg-options "-O0 -march=rv32e -mabi=ilp32e -fomit-frame-pointer" } */
+
+int getInt();
+void PrintInts(int);
+
+int callPrintInts()
+{
+  int i = getInt();
+  PrintInts(i);
+  return i;
+}
+
+/* { dg-final { scan-assembler-not "addi	sp,sp,-16" } } */
-- 
2.17.1

Re: [PATCH] RISC-V: decouple stack allocation for rv32e w/o save-restore.

[Jeff Law]

On 4/21/23 04:07, Fei Gao wrote:
> Currently in rv32e, stack allocation for GPR callee-saved registers is
> always 12 bytes w/o save-restore. Actually, for the case without save-restore,
> less stack memory can be reserved. This patch decouples stack allocation for
> rv32e w/o save-restore and makes riscv_compute_frame_info more readable.
> 
> output of testcase rv32e_stack.c
> before patch:
> 	addi	sp,sp,-16
> 	sw	ra,12(sp)
> 	call	getInt
> 	sw	a0,0(sp)
> 	lw	a0,0(sp)
> 	call	PrintInts
> 	lw	a5,0(sp)
> 	mv	a0,a5
> 	lw	ra,12(sp)
> 	addi	sp,sp,16
> 	jr	ra
> 
> after patch:
> 	addi	sp,sp,-8
> 	sw	ra,4(sp)
> 	call	getInt
> 	sw	a0,0(sp)
> 	lw	a0,0(sp)
> 	call	PrintInts
> 	lw	a5,0(sp)
> 	mv	a0,a5
> 	lw	ra,4(sp)
> 	addi	sp,sp,8
> 	jr	ra
> 
> gcc/ChangeLog:
> 
>          * config/riscv/riscv.cc (riscv_forbid_save_libcall): helper function for riscv_use_save_libcall.
>          (riscv_use_save_libcall): call riscv_forbid_save_libcall.
>          (riscv_compute_frame_info): restructure to decouple stack allocation for rv32e w/o save-restore.
> 
> gcc/testsuite/ChangeLog:
> 
>          * gcc.target/riscv/rv32e_stack.c: New test.
> ---
>   gcc/config/riscv/riscv.cc                    | 57 ++++++++++++--------
>   gcc/testsuite/gcc.target/riscv/rv32e_stack.c | 14 +++++
>   2 files changed, 49 insertions(+), 22 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.target/riscv/rv32e_stack.c
> 
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 5d2550871c7..6ccdfe96fe7 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -4772,12 +4772,26 @@ riscv_save_reg_p (unsigned int regno)
>     return false;
>   }
>   
> +/* Determine whether to disable GPR save/restore routines.  */
> +static bool
> +riscv_forbid_save_libcall (void)
I would suggest something like this for the function comment:

/* Return TRUE if a libcall to save/restore GPRs should be
    avoided.  FALSE otherwise.  */

I would also change the name from "forbid" to "avoid".


With those changes I think this will be ready for the trunk.  So repost 
after those changes and I'll get it pushed into the trunk.

Thanks,
Jeff