| Message ID | 20231114141455.24465-1-kito.cheng@sifive.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Save/restore ra register correctly [PR112478] | expand |
On Tue, Nov 14, 2023 at 3:15 PM Kito Cheng <kito.cheng@sifive.com> wrote: > > We set ra to fixed register now, but we still need to save/restore that at > prologue/epilogue if that has used. So before 71f906498ada9 $ra was neither a fixed nor a used register. Therefore, riscv_save_reg_p returned true in the first test (not global reg, not used_or_fixed, and ever_live_p). After this commit, this does not happen anymore, because the test for not used_or_fixed fails and we don't test for ever_live_p in the following. And this patch restores this behavior. Reviewed-by: Christoph Müllner <christoph.muellner@vrull.eu> Tested-by: Christoph Müllner <christoph.muellner@vrull.eu> > > gcc/ChangeLog: > > PR target/112478 > * config/riscv/riscv.cc (riscv_save_return_addr_reg_p): Check ra > is ever lived. > > gcc/testsuite/gcc/ChangeLog: > > PR target/112478 > * riscv/pr112478.c: New. > --- > gcc/config/riscv/riscv.cc | 4 ++++ > gcc/testsuite/gcc.target/riscv/pr112478.c | 8 ++++++++ > 2 files changed, 12 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/riscv/pr112478.c > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index ecee7eb4727..f09c4066903 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -5802,6 +5802,10 @@ riscv_save_return_addr_reg_p (void) > if (riscv_far_jump_used_p ()) > return true; > > + /* We need to save it if anyone has used that. */ > + if (df_regs_ever_live_p (RETURN_ADDR_REGNUM)) > + return true; > + > /* Need not to use ra for leaf when frame pointer is turned off by > option whatever the omit-leaf-frame's value. */ > if (frame_pointer_needed && crtl->is_leaf > diff --git a/gcc/testsuite/gcc.target/riscv/pr112478.c b/gcc/testsuite/gcc.target/riscv/pr112478.c > new file mode 100644 > index 00000000000..0bbde20b71b > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/pr112478.c > @@ -0,0 +1,8 @@ > +/* { dg-do compile } */ > +/* { dg-options "-ffat-lto-objects" } */ > + > +void foo() { > + asm volatile("# " : ::"ra"); > +} > + > +/* { dg-final { scan-assembler "s(w|d)\[ \t\]*ra" } } */ > -- > 2.40.1 >
Committed, thanks for the review and test :) On Thu, Nov 16, 2023 at 7:19 AM Christoph Müllner <christoph.muellner@vrull.eu> wrote: > > On Tue, Nov 14, 2023 at 3:15 PM Kito Cheng <kito.cheng@sifive.com> wrote: > > > > We set ra to fixed register now, but we still need to save/restore that at > > prologue/epilogue if that has used. > > So before 71f906498ada9 $ra was neither a fixed nor a used register. > Therefore, riscv_save_reg_p returned true in the first test (not global reg, > not used_or_fixed, and ever_live_p). > After this commit, this does not happen anymore, because the test for > not used_or_fixed fails and we don't test for ever_live_p in the following. > And this patch restores this behavior. > > Reviewed-by: Christoph Müllner <christoph.muellner@vrull.eu> > Tested-by: Christoph Müllner <christoph.muellner@vrull.eu> > > > > > gcc/ChangeLog: > > > > PR target/112478 > > * config/riscv/riscv.cc (riscv_save_return_addr_reg_p): Check ra > > is ever lived. > > > > gcc/testsuite/gcc/ChangeLog: > > > > PR target/112478 > > * riscv/pr112478.c: New. > > --- > > gcc/config/riscv/riscv.cc | 4 ++++ > > gcc/testsuite/gcc.target/riscv/pr112478.c | 8 ++++++++ > > 2 files changed, 12 insertions(+) > > create mode 100644 gcc/testsuite/gcc.target/riscv/pr112478.c > > > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > > index ecee7eb4727..f09c4066903 100644 > > --- a/gcc/config/riscv/riscv.cc > > +++ b/gcc/config/riscv/riscv.cc > > @@ -5802,6 +5802,10 @@ riscv_save_return_addr_reg_p (void) > > if (riscv_far_jump_used_p ()) > > return true; > > > > + /* We need to save it if anyone has used that. */ > > + if (df_regs_ever_live_p (RETURN_ADDR_REGNUM)) > > + return true; > > + > > /* Need not to use ra for leaf when frame pointer is turned off by > > option whatever the omit-leaf-frame's value. */ > > if (frame_pointer_needed && crtl->is_leaf > > diff --git a/gcc/testsuite/gcc.target/riscv/pr112478.c b/gcc/testsuite/gcc.target/riscv/pr112478.c > > new file mode 100644 > > index 00000000000..0bbde20b71b > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/riscv/pr112478.c > > @@ -0,0 +1,8 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-ffat-lto-objects" } */ > > + > > +void foo() { > > + asm volatile("# " : ::"ra"); > > +} > > + > > +/* { dg-final { scan-assembler "s(w|d)\[ \t\]*ra" } } */ > > -- > > 2.40.1 > >
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index ecee7eb4727..f09c4066903 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -5802,6 +5802,10 @@ riscv_save_return_addr_reg_p (void) if (riscv_far_jump_used_p ()) return true; + /* We need to save it if anyone has used that. */ + if (df_regs_ever_live_p (RETURN_ADDR_REGNUM)) + return true; + /* Need not to use ra for leaf when frame pointer is turned off by option whatever the omit-leaf-frame's value. */ if (frame_pointer_needed && crtl->is_leaf diff --git a/gcc/testsuite/gcc.target/riscv/pr112478.c b/gcc/testsuite/gcc.target/riscv/pr112478.c new file mode 100644 index 00000000000..0bbde20b71b --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr112478.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-ffat-lto-objects" } */ + +void foo() { + asm volatile("# " : ::"ra"); +} + +/* { dg-final { scan-assembler "s(w|d)\[ \t\]*ra" } } */