Re: [PATCH] fix pdp11_expand_epilogue (PR target/107841)
On Thu, 13 Jul 2023, Jeff Law via Gcc-patches wrote: > > Question for the experts: how is this handled? Do I need to apply this > > change to my workspace and commit it, with Mikael as the change author? > That's what I usually do for someone without write access. commit it locally > using the --author flag, then push. FWIW `git am' taking the e-mail containing a change on stdin is I believe the most straightforward way, as long as the submission has been properly formatted. The command automatically sets the author correctly, picking the `From:' entry from the e-mail headers or the first line of the e-mail body as appropriate, sets the author date from the `Date:' header, picks up the contents of the `Subject:' header as the change heading and the body of the e-mail sans any `From:' entry as the change description, and strips any discussion between the non-commit delimiter (`--') and the patch itself. The command was designed for maintainers to import changes submitted by e-mail so it does what expected. Any issues with an imported change can then be addressed with `git commit --amend', etc. HTH, Maciej
Re: [PATCH] fix pdp11_expand_epilogue (PR target/107841)
> On Jul 13, 2023, at 12:47 PM, Mikael Pettersson wrote:
>
> If the stack frame only contains an alloca area, then
> pdp11_expand_epilogue fails to deallocate it, resulting
> in callee-saved registers and the return address being
> restored from the wrong stack slots. Fixed by adding
> || cfun->calls_alloca to the condition for deallocating
> the frame.
>
> Tested with a cross to pdp11-unknown-aout.
>
> Ok for master? (Note: I don't have commit rights.)
>
> gcc/
>
> PR target/107841
> * config/pdp11/pdp11.c (pdp11_expand_epilogue): Also
> deallocate alloca-only frame.
>
> gcc/testsuite/
>
> PR target/107841
> * gcc.target/pdp11/pr107841.c: New test.
> ---
> gcc/config/pdp11/pdp11.cc | 2 +-
> gcc/testsuite/gcc.target/pdp11/pr107841.c | 12
> 2 files changed, 13 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/pdp11/pr107841.c
>
> diff --git a/gcc/config/pdp11/pdp11.cc b/gcc/config/pdp11/pdp11.cc
> index f6dd841f184..311a1d225e0 100644
> --- a/gcc/config/pdp11/pdp11.cc
> +++ b/gcc/config/pdp11/pdp11.cc
> @@ -393,7 +393,7 @@ pdp11_expand_epilogue (void)
> rtx x, reg, via_ac = NULL;
>
> /* Deallocate the local variables. */
> - if (fsize)
> + if (fsize || cfun->calls_alloca)
> {
> if (frame_pointer_needed)
> {
> diff --git a/gcc/testsuite/gcc.target/pdp11/pr107841.c
> b/gcc/testsuite/gcc.target/pdp11/pr107841.c
> new file mode 100644
> index 000..a363c468b0b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/pdp11/pr107841.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +/* Verify that the stack frame is deallocated using the frame pointer. */
> +
> +void qq (int a)
> +{
> +char *s = __builtin_alloca (128);
> +__builtin_sprintf (s, "qq %d", 3);
> +}
> +
> +/* { dg-final { scan-assembler "mov\tr5,sp" } } */
> --
> 2.41.0
>
Thanks Mikael. I committed this fix.
paul
Re: [PATCH] fix pdp11_expand_epilogue (PR target/107841)
On 7/13/23 11:46, Paul Koning via Gcc-patches wrote: On Jul 13, 2023, at 12:47 PM, Mikael Pettersson wrote: If the stack frame only contains an alloca area, then pdp11_expand_epilogue fails to deallocate it, resulting in callee-saved registers and the return address being restored from the wrong stack slots. Fixed by adding || cfun->calls_alloca to the condition for deallocating the frame. Tested with a cross to pdp11-unknown-aout. Ok for master? (Note: I don't have commit rights.) Yes, thank you! Question for the experts: how is this handled? Do I need to apply this change to my workspace and commit it, with Mikael as the change author? That's what I usually do for someone without write access. commit it locally using the --author flag, then push. jeff
Re: [PATCH] fix pdp11_expand_epilogue (PR target/107841)
> On Jul 13, 2023, at 12:47 PM, Mikael Pettersson wrote:
>
> If the stack frame only contains an alloca area, then
> pdp11_expand_epilogue fails to deallocate it, resulting
> in callee-saved registers and the return address being
> restored from the wrong stack slots. Fixed by adding
> || cfun->calls_alloca to the condition for deallocating
> the frame.
>
> Tested with a cross to pdp11-unknown-aout.
>
> Ok for master? (Note: I don't have commit rights.)
Yes, thank you!
Question for the experts: how is this handled? Do I need to apply this change
to my workspace and commit it, with Mikael as the change author?
paul
>
> gcc/
>
> PR target/107841
> * config/pdp11/pdp11.c (pdp11_expand_epilogue): Also
> deallocate alloca-only frame.
>
> gcc/testsuite/
>
> PR target/107841
> * gcc.target/pdp11/pr107841.c: New test.
> ---
> gcc/config/pdp11/pdp11.cc | 2 +-
> gcc/testsuite/gcc.target/pdp11/pr107841.c | 12
> 2 files changed, 13 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/pdp11/pr107841.c
>
> diff --git a/gcc/config/pdp11/pdp11.cc b/gcc/config/pdp11/pdp11.cc
> index f6dd841f184..311a1d225e0 100644
> --- a/gcc/config/pdp11/pdp11.cc
> +++ b/gcc/config/pdp11/pdp11.cc
> @@ -393,7 +393,7 @@ pdp11_expand_epilogue (void)
> rtx x, reg, via_ac = NULL;
>
> /* Deallocate the local variables. */
> - if (fsize)
> + if (fsize || cfun->calls_alloca)
> {
> if (frame_pointer_needed)
> {
> diff --git a/gcc/testsuite/gcc.target/pdp11/pr107841.c
> b/gcc/testsuite/gcc.target/pdp11/pr107841.c
> new file mode 100644
> index 000..a363c468b0b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/pdp11/pr107841.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +/* Verify that the stack frame is deallocated using the frame pointer. */
> +
> +void qq (int a)
> +{
> +char *s = __builtin_alloca (128);
> +__builtin_sprintf (s, "qq %d", 3);
> +}
> +
> +/* { dg-final { scan-assembler "mov\tr5,sp" } } */
> --
> 2.41.0
>
[PATCH] fix pdp11_expand_epilogue (PR target/107841)
If the stack frame only contains an alloca area, then
pdp11_expand_epilogue fails to deallocate it, resulting
in callee-saved registers and the return address being
restored from the wrong stack slots. Fixed by adding
|| cfun->calls_alloca to the condition for deallocating
the frame.
Tested with a cross to pdp11-unknown-aout.
Ok for master? (Note: I don't have commit rights.)
gcc/
PR target/107841
* config/pdp11/pdp11.c (pdp11_expand_epilogue): Also
deallocate alloca-only frame.
gcc/testsuite/
PR target/107841
* gcc.target/pdp11/pr107841.c: New test.
---
gcc/config/pdp11/pdp11.cc | 2 +-
gcc/testsuite/gcc.target/pdp11/pr107841.c | 12
2 files changed, 13 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/pdp11/pr107841.c
diff --git a/gcc/config/pdp11/pdp11.cc b/gcc/config/pdp11/pdp11.cc
index f6dd841f184..311a1d225e0 100644
--- a/gcc/config/pdp11/pdp11.cc
+++ b/gcc/config/pdp11/pdp11.cc
@@ -393,7 +393,7 @@ pdp11_expand_epilogue (void)
rtx x, reg, via_ac = NULL;
/* Deallocate the local variables. */
- if (fsize)
+ if (fsize || cfun->calls_alloca)
{
if (frame_pointer_needed)
{
diff --git a/gcc/testsuite/gcc.target/pdp11/pr107841.c
b/gcc/testsuite/gcc.target/pdp11/pr107841.c
new file mode 100644
index 000..a363c468b0b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/pdp11/pr107841.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+/* Verify that the stack frame is deallocated using the frame pointer. */
+
+void qq (int a)
+{
+char *s = __builtin_alloca (128);
+__builtin_sprintf (s, "qq %d", 3);
+}
+
+/* { dg-final { scan-assembler "mov\tr5,sp" } } */
--
2.41.0
