diff mbox series

RISC-V: Disable remove unneeded save-restore call optimization if there are any arguments on stack.

Message ID 20200707072801.104843-1-kito.cheng@sifive.com
State New
Headers show
Series RISC-V: Disable remove unneeded save-restore call optimization if there are any arguments on stack. | expand

Commit Message

Kito Cheng July 7, 2020, 7:28 a.m. UTC 
- This optimization will adjust stack, but it not check/update other
   stack pointer use-site, the example is when the arguments put on
   stack, the offset become wrong after optimization.

 - However adjust stack frame usage after register allocation could be
   error prone, so we decide to turn off this optimization for such case.

 - Ye-Ting Kuo report this issue on github:
   https://github.com/riscv/riscv-gcc/pull/192

gcc/ChangeLog:

	* config/riscv/riscv-sr.c (riscv_remove_unneeded_save_restore_calls):
	Abort if any arguments on stack.

gcc/testsuite/ChangeLog

	* gcc.target/riscv/save-restore-9.c: New.
---
 gcc/config/riscv/riscv-sr.c                   |  6 +++++
 .../gcc.target/riscv/save-restore-9.c         | 23 +++++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/save-restore-9.c

Comments

Jim Wilson July 8, 2020, 7:39 p.m. UTC | #1 
On Tue, Jul 7, 2020 at 12:28 AM Kito Cheng <kito.cheng@sifive.com> wrote:
> gcc/ChangeLog:
>         * config/riscv/riscv-sr.c (riscv_remove_unneeded_save_restore_calls):
>         Abort if any arguments on stack.
> gcc/testsuite/ChangeLog
>         * gcc.target/riscv/save-restore-9.c: New.

Looks good to me.

Jim
Kito Cheng July 9, 2020, 6:53 a.m. UTC | #2 
Committed, thanks.

On Thu, Jul 9, 2020 at 3:39 AM Jim Wilson <jimw@sifive.com> wrote:
>
> On Tue, Jul 7, 2020 at 12:28 AM Kito Cheng <kito.cheng@sifive.com> wrote:
> > gcc/ChangeLog:
> >         * config/riscv/riscv-sr.c (riscv_remove_unneeded_save_restore_calls):
> >         Abort if any arguments on stack.
> > gcc/testsuite/ChangeLog
> >         * gcc.target/riscv/save-restore-9.c: New.
>
> Looks good to me.
>
> Jim
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv-sr.c b/gcc/config/riscv/riscv-sr.c
index 9af50ef028e3..694f90c1583a 100644
--- a/gcc/config/riscv/riscv-sr.c
+++ b/gcc/config/riscv/riscv-sr.c
@@ -244,6 +244,12 @@  check_for_no_return_call (rtx_insn *prologue)
 void
 riscv_remove_unneeded_save_restore_calls (void)
 {
+  /* We'll adjust stack size after this optimization, that require update every
+     sp use site, which could be unsafe, so we decide to turn off this
+     optimization if there are any arguments put on stack.  */
+  if (crtl->args.size != 0)
+    return;
+
   /* Will point to the first instruction of the function body, after the
      prologue end note.  */
   rtx_insn *body = NULL;
diff --git a/gcc/testsuite/gcc.target/riscv/save-restore-9.c b/gcc/testsuite/gcc.target/riscv/save-restore-9.c
new file mode 100644
index 000000000000..2567daeb376b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/save-restore-9.c
@@ -0,0 +1,23 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 -msave-restore" } */
+
+int
+__attribute__((noinline,noclone))
+foo (int u)
+{
+  return u + 1;
+}
+
+int
+__attribute__((noinline,noclone))
+bar (int a, int b, int c, int d, int e, int f, int g, int h, int u)
+{
+  return foo (u);
+}
+
+int main()
+{
+  if (bar (1, 2, 3, 4, 5, 6, 7, 8, 9) != 10)
+    __builtin_abort();
+  return 0;
+}