public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR 95481: tail call fails with empty struct types
@ 2021-05-31  6:30 apinski
  2021-05-31  6:44 ` Richard Biener
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: apinski @ 2021-05-31  6:30 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

From: Andrew Pinski <apinski@marvell.com>

The problem here is we don't have an assignment type any more
for zero-length structs as they were removed during gimplifcation.
This adds a special case where the assignment var does not exist
and the return decl is zero-length.

OK? Tested on aarch64-linux-gnu with no regressions.

Thanks,
Andrew Pinski

gcc/ChangeLog:
	* tree-tailcall.c (zero_sized_decl): New function.
	(find_tail_calls): Handle zero sized return decls.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/tailcall-10.c: New test.
	* gcc.dg/tree-ssa/tailcall-11.c: New test.
	* gcc.dg/tree-ssa/tailcall-12.c: New test.
	* gcc.dg/tree-ssa/tailcall-13.c: New test.
	* gcc.dg/tree-ssa/tailrecursion-8.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/tailcall-10.c   | 12 ++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/tailcall-11.c   | 12 ++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/tailcall-12.c   | 12 ++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/tailcall-13.c   | 15 +++++++++++++
 .../gcc.dg/tree-ssa/tailrecursion-8.c         | 11 ++++++++++
 gcc/tree-tailcall.c                           | 22 +++++++++++++++++--
 6 files changed, 82 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/tailcall-10.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/tailcall-11.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/tailcall-12.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/tailcall-13.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/tailrecursion-8.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-10.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-10.c
new file mode 100644
index 00000000000..484dcc125fc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-10.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-tailc-details" } */
+
+struct A {};
+
+struct A goo(void);
+struct A foo(void)
+{
+  return goo();
+}
+
+/* { dg-final { scan-tree-dump-times "Found tail call" 1 "tailc"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-11.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-11.c
new file mode 100644
index 00000000000..36e441775ff
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-11.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-tailc-details" } */
+
+struct A {};
+
+void goo(void);
+struct A foo(void)
+{
+  goo();
+}
+
+/* { dg-final { scan-tree-dump-times "Found tail call" 1 "tailc"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-12.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-12.c
new file mode 100644
index 00000000000..0eeb3ab2794
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-12.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-tailc-details" } */
+
+struct A {};
+
+struct A goo(void);
+void foo(void)
+{
+  goo();
+}
+
+/* { dg-final { scan-tree-dump-times "Found tail call" 1 "tailc"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-13.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-13.c
new file mode 100644
index 00000000000..855b3312ef4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-13.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-tailc-details" } */
+
+struct A {};
+struct B{};
+
+struct B goo(void);
+struct A foo(void)
+{
+  struct A a;
+  goo();
+  return a;
+}
+
+/* { dg-final { scan-tree-dump-times "Found tail call" 1 "tailc"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailrecursion-8.c b/gcc/testsuite/gcc.dg/tree-ssa/tailrecursion-8.c
new file mode 100644
index 00000000000..ecde499a06b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/tailrecursion-8.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -foptimize-sibling-calls -fdump-tree-tailr1-details" } */
+
+struct A {};
+
+struct A foo()
+{
+  return foo();
+}
+
+/* { dg-final { scan-tree-dump-times "Eliminated tail recursion" 1 "tailr1"} } */
diff --git a/gcc/tree-tailcall.c b/gcc/tree-tailcall.c
index e866f7272ed..ebb42f7aa9b 100644
--- a/gcc/tree-tailcall.c
+++ b/gcc/tree-tailcall.c
@@ -410,6 +410,22 @@ propagate_through_phis (tree var, edge e)
 static live_vars_map *live_vars;
 static vec<bitmap_head> live_vars_vec;
 
+/* Return true if decl is zero sized.  */
+
+static bool
+zero_sized_decl (const_tree decl)
+{
+  if (!decl)
+    return true;
+
+  tree type = TREE_TYPE (decl);
+  if (AGGREGATE_TYPE_P (type) && TYPE_SIZE (type)
+      && integer_zerop (TYPE_SIZE (type)))
+    return true;
+  return false;
+}
+
+
 /* Finds tailcalls falling into basic block BB. The list of found tailcalls is
    added to the start of RET.  */
 
@@ -710,9 +726,11 @@ find_tail_calls (basic_block bb, struct tailcall **ret)
   ret_var = gimple_return_retval (as_a <greturn *> (stmt));
 
   /* We may proceed if there either is no return value, or the return value
-     is identical to the call's return.  */
+     is identical to the call's return or if the return decl is a zero sized
+     variable and the call's return was not assigned. */
   if (ret_var
-      && (ret_var != ass_var))
+      && (ret_var != ass_var
+	  && !(zero_sized_decl (ret_var) && !ass_var)))
     return;
 
   /* If this is not a tail recursive call, we cannot handle addends or
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR 95481: tail call fails with empty struct types
  2021-05-31  6:30 [PATCH] Fix PR 95481: tail call fails with empty struct types apinski
@ 2021-05-31  6:44 ` Richard Biener
  2021-05-31  8:25 ` Jakub Jelinek
  2021-05-31 12:16 ` H.J. Lu
  2 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2021-05-31  6:44 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: GCC Patches

On Mon, May 31, 2021 at 8:36 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> The problem here is we don't have an assignment type any more
> for zero-length structs as they were removed during gimplifcation.
> This adds a special case where the assignment var does not exist
> and the return decl is zero-length.
>
> OK? Tested on aarch64-linux-gnu with no regressions.
>
> Thanks,
> Andrew Pinski
>
> gcc/ChangeLog:
>         * tree-tailcall.c (zero_sized_decl): New function.
>         (find_tail_calls): Handle zero sized return decls.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/tailcall-10.c: New test.
>         * gcc.dg/tree-ssa/tailcall-11.c: New test.
>         * gcc.dg/tree-ssa/tailcall-12.c: New test.
>         * gcc.dg/tree-ssa/tailcall-13.c: New test.
>         * gcc.dg/tree-ssa/tailrecursion-8.c: New test.
> ---
>  gcc/testsuite/gcc.dg/tree-ssa/tailcall-10.c   | 12 ++++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/tailcall-11.c   | 12 ++++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/tailcall-12.c   | 12 ++++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/tailcall-13.c   | 15 +++++++++++++
>  .../gcc.dg/tree-ssa/tailrecursion-8.c         | 11 ++++++++++
>  gcc/tree-tailcall.c                           | 22 +++++++++++++++++--
>  6 files changed, 82 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/tailcall-10.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/tailcall-11.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/tailcall-12.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/tailcall-13.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/tailrecursion-8.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-10.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-10.c
> new file mode 100644
> index 00000000000..484dcc125fc
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-10.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-tailc-details" } */
> +
> +struct A {};
> +
> +struct A goo(void);
> +struct A foo(void)
> +{
> +  return goo();
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Found tail call" 1 "tailc"} } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-11.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-11.c
> new file mode 100644
> index 00000000000..36e441775ff
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-11.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-tailc-details" } */
> +
> +struct A {};
> +
> +void goo(void);
> +struct A foo(void)
> +{
> +  goo();
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Found tail call" 1 "tailc"} } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-12.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-12.c
> new file mode 100644
> index 00000000000..0eeb3ab2794
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-12.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-tailc-details" } */
> +
> +struct A {};
> +
> +struct A goo(void);
> +void foo(void)
> +{
> +  goo();
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Found tail call" 1 "tailc"} } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-13.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-13.c
> new file mode 100644
> index 00000000000..855b3312ef4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-13.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-tailc-details" } */
> +
> +struct A {};
> +struct B{};
> +
> +struct B goo(void);
> +struct A foo(void)
> +{
> +  struct A a;
> +  goo();
> +  return a;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Found tail call" 1 "tailc"} } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailrecursion-8.c b/gcc/testsuite/gcc.dg/tree-ssa/tailrecursion-8.c
> new file mode 100644
> index 00000000000..ecde499a06b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/tailrecursion-8.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -foptimize-sibling-calls -fdump-tree-tailr1-details" } */
> +
> +struct A {};
> +
> +struct A foo()
> +{
> +  return foo();
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Eliminated tail recursion" 1 "tailr1"} } */
> diff --git a/gcc/tree-tailcall.c b/gcc/tree-tailcall.c
> index e866f7272ed..ebb42f7aa9b 100644
> --- a/gcc/tree-tailcall.c
> +++ b/gcc/tree-tailcall.c
> @@ -410,6 +410,22 @@ propagate_through_phis (tree var, edge e)
>  static live_vars_map *live_vars;
>  static vec<bitmap_head> live_vars_vec;
>
> +/* Return true if decl is zero sized.  */
> +
> +static bool
> +zero_sized_decl (const_tree decl)
> +{
> +  if (!decl)
> +    return true;
> +
> +  tree type = TREE_TYPE (decl);
> +  if (AGGREGATE_TYPE_P (type) && TYPE_SIZE (type)
> +      && integer_zerop (TYPE_SIZE (type)))
> +    return true;
> +  return false;
> +}
> +
> +
>  /* Finds tailcalls falling into basic block BB. The list of found tailcalls is
>     added to the start of RET.  */
>
> @@ -710,9 +726,11 @@ find_tail_calls (basic_block bb, struct tailcall **ret)
>    ret_var = gimple_return_retval (as_a <greturn *> (stmt));
>
>    /* We may proceed if there either is no return value, or the return value
> -     is identical to the call's return.  */
> +     is identical to the call's return or if the return decl is a zero sized
> +     variable and the call's return was not assigned. */
>    if (ret_var
> -      && (ret_var != ass_var))
> +      && (ret_var != ass_var
> +         && !(zero_sized_decl (ret_var) && !ass_var)))
>      return;

I wonder if is_empty_type () would work here, it should be slightly
more powerful
plus it avoids adding a function that feels like it should be already present ;)

Thanks,
Richard,

>
>    /* If this is not a tail recursive call, we cannot handle addends or
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR 95481: tail call fails with empty struct types
  2021-05-31  6:30 [PATCH] Fix PR 95481: tail call fails with empty struct types apinski
  2021-05-31  6:44 ` Richard Biener
@ 2021-05-31  8:25 ` Jakub Jelinek
  2021-05-31  9:13   ` Richard Biener
  2021-05-31 12:16 ` H.J. Lu
  2 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2021-05-31  8:25 UTC (permalink / raw)
  To: apinski; +Cc: gcc-patches

On Sun, May 30, 2021 at 11:30:25PM -0700, apinski--- via Gcc-patches wrote:
> +static bool
> +zero_sized_decl (const_tree decl)
> +{
> +  if (!decl)
> +    return true;
> +
> +  tree type = TREE_TYPE (decl);
> +  if (AGGREGATE_TYPE_P (type) && TYPE_SIZE (type)
> +      && integer_zerop (TYPE_SIZE (type)))

Shouldn't this be instead
  if (AGGREGATE_TYPE_P (type)
      && !TREE_ADDRESSABLE (type)
      && is_empty_type (type))
?
It shouldn't matter if the type has zero size (e.g. typically in C) or
non-zero size (typically in C++), but that returning it is a nop, no bits
need to be copied anywhere.

	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR 95481: tail call fails with empty struct types
  2021-05-31  8:25 ` Jakub Jelinek
@ 2021-05-31  9:13   ` Richard Biener
  2021-05-31  9:15     ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2021-05-31  9:13 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Andrew Pinski, GCC Patches

On Mon, May 31, 2021 at 10:29 AM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Sun, May 30, 2021 at 11:30:25PM -0700, apinski--- via Gcc-patches wrote:
> > +static bool
> > +zero_sized_decl (const_tree decl)
> > +{
> > +  if (!decl)
> > +    return true;
> > +
> > +  tree type = TREE_TYPE (decl);
> > +  if (AGGREGATE_TYPE_P (type) && TYPE_SIZE (type)
> > +      && integer_zerop (TYPE_SIZE (type)))
>
> Shouldn't this be instead
>   if (AGGREGATE_TYPE_P (type)
>       && !TREE_ADDRESSABLE (type)
>       && is_empty_type (type))
> ?
> It shouldn't matter if the type has zero size (e.g. typically in C) or
> non-zero size (typically in C++), but that returning it is a nop, no bits
> need to be copied anywhere.

But does !TREE_ADDRESSABLE matter?

>         Jakub
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR 95481: tail call fails with empty struct types
  2021-05-31  9:13   ` Richard Biener
@ 2021-05-31  9:15     ` Jakub Jelinek
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2021-05-31  9:15 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew Pinski, GCC Patches

On Mon, May 31, 2021 at 11:13:22AM +0200, Richard Biener wrote:
> On Mon, May 31, 2021 at 10:29 AM Jakub Jelinek via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > On Sun, May 30, 2021 at 11:30:25PM -0700, apinski--- via Gcc-patches wrote:
> > > +static bool
> > > +zero_sized_decl (const_tree decl)
> > > +{
> > > +  if (!decl)
> > > +    return true;
> > > +
> > > +  tree type = TREE_TYPE (decl);
> > > +  if (AGGREGATE_TYPE_P (type) && TYPE_SIZE (type)
> > > +      && integer_zerop (TYPE_SIZE (type)))
> >
> > Shouldn't this be instead
> >   if (AGGREGATE_TYPE_P (type)
> >       && !TREE_ADDRESSABLE (type)
> >       && is_empty_type (type))
> > ?
> > It shouldn't matter if the type has zero size (e.g. typically in C) or
> > non-zero size (typically in C++), but that returning it is a nop, no bits
> > need to be copied anywhere.
> 
> But does !TREE_ADDRESSABLE matter?

Likely the FEs should pass it by hidden reference in that case already,
true.

	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR 95481: tail call fails with empty struct types
  2021-05-31  6:30 [PATCH] Fix PR 95481: tail call fails with empty struct types apinski
  2021-05-31  6:44 ` Richard Biener
  2021-05-31  8:25 ` Jakub Jelinek
@ 2021-05-31 12:16 ` H.J. Lu
  2 siblings, 0 replies; 6+ messages in thread
From: H.J. Lu @ 2021-05-31 12:16 UTC (permalink / raw)
  To: apinski; +Cc: GCC Patches

On Sun, May 30, 2021 at 11:35 PM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> The problem here is we don't have an assignment type any more
> for zero-length structs as they were removed during gimplifcation.
> This adds a special case where the assignment var does not exist
> and the return decl is zero-length.
>
> OK? Tested on aarch64-linux-gnu with no regressions.
>
> Thanks,
> Andrew Pinski
>
> gcc/ChangeLog:
>         * tree-tailcall.c (zero_sized_decl): New function.
>         (find_tail_calls): Handle zero sized return decls.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/tailcall-10.c: New test.
>         * gcc.dg/tree-ssa/tailcall-11.c: New test.
>         * gcc.dg/tree-ssa/tailcall-12.c: New test.
>         * gcc.dg/tree-ssa/tailcall-13.c: New test.
>         * gcc.dg/tree-ssa/tailrecursion-8.c: New test.

Please mention PR 95481 in the commit log.

Thanks.

H.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-05-31 12:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-31  6:30 [PATCH] Fix PR 95481: tail call fails with empty struct types apinski
2021-05-31  6:44 ` Richard Biener
2021-05-31  8:25 ` Jakub Jelinek
2021-05-31  9:13   ` Richard Biener
2021-05-31  9:15     ` Jakub Jelinek
2021-05-31 12:16 ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).