Patchwork Avoid doing empty bb ehcleanup move if PHI on the landing pad has other uses (PR tree-optimization/47355)

login
register
mail settings
Submitter Jakub Jelinek
Date Jan. 20, 2011, 4:29 p.m.
Message ID <20110120162906.GY2724@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/79721/
State New
Headers show

Comments

Jakub Jelinek - Jan. 20, 2011, 4:29 p.m.
Hi!

On the attached testcase we have something like:
  # names$_storage_30 = PHI <names$_storage_22(D)(14), names$_storage_1(5), names$_storage_22(D)(13)>
<L1>:
  goto <bb 17>;

<bb 16>:
  special_destroy (names$_storage_29);
  names$_storage_21 = names$_storage_29 + 8;
  names$_size_23 = names$_size_7 + -1;
  
<bb 17>:
  # names$_storage_29 = PHI <names$_storage_30(15), names$_storage_21(16)>
  # names$_size_7 = PHI <names$_size_38(D)(15), names$_size_23(16)>
  if (names$_size_7 != 0)
    goto <bb 16>;
  else
    goto <bb 18>;

<bb 18>:
  D.2159_25 = (long unsigned int) names$_size_38(D);
  D.2160_19 = D.2159_25 * 8;
  names$_storage_26 = names$_storage_30 + D.2160_19;
  zfree (names$_storage_26);

<L3>:
  resx 1
and ehcleanup would like to move the landing pad from the L1 block
to bb 17.  It sees names$_storage_30 is used in bb 17 PHI argument
from that edge and just goes on with change, which means
names$_storage_30 is no longer defined, but one use in bb 18
is left around.

In this case we can't even replace _30 in bb18 with _29,
because that value could be incremented in the loop before it.

This patch fixes it by not moving the landing pad if there are other
uses of the old PHI result.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2011-01-20  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/47355
	* tree-eh.c (cleanup_empty_eh_merge_phis): Give up if
	NOP has more than a single use.

	* g++.dg/opt/pr47355.C: New test.


	Jakub
Richard Guenther - Jan. 21, 2011, 10:26 a.m.
On Thu, Jan 20, 2011 at 5:29 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On the attached testcase we have something like:
>  # names$_storage_30 = PHI <names$_storage_22(D)(14), names$_storage_1(5), names$_storage_22(D)(13)>
> <L1>:
>  goto <bb 17>;
>
> <bb 16>:
>  special_destroy (names$_storage_29);
>  names$_storage_21 = names$_storage_29 + 8;
>  names$_size_23 = names$_size_7 + -1;
>
> <bb 17>:
>  # names$_storage_29 = PHI <names$_storage_30(15), names$_storage_21(16)>
>  # names$_size_7 = PHI <names$_size_38(D)(15), names$_size_23(16)>
>  if (names$_size_7 != 0)
>    goto <bb 16>;
>  else
>    goto <bb 18>;
>
> <bb 18>:
>  D.2159_25 = (long unsigned int) names$_size_38(D);
>  D.2160_19 = D.2159_25 * 8;
>  names$_storage_26 = names$_storage_30 + D.2160_19;
>  zfree (names$_storage_26);
>
> <L3>:
>  resx 1
> and ehcleanup would like to move the landing pad from the L1 block
> to bb 17.  It sees names$_storage_30 is used in bb 17 PHI argument
> from that edge and just goes on with change, which means
> names$_storage_30 is no longer defined, but one use in bb 18
> is left around.
>
> In this case we can't even replace _30 in bb18 with _29,
> because that value could be incremented in the loop before it.
>
> This patch fixes it by not moving the landing pad if there are other
> uses of the old PHI result.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2011-01-20  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/47355
>        * tree-eh.c (cleanup_empty_eh_merge_phis): Give up if
>        NOP has more than a single use.
>
>        * g++.dg/opt/pr47355.C: New test.
>
> --- gcc/tree-eh.c.jj    2011-01-18 18:16:11.000000000 +0100
> +++ gcc/tree-eh.c       2011-01-20 11:11:48.000000000 +0100
> @@ -3552,6 +3552,9 @@ cleanup_empty_eh_merge_phis (basic_block
>       /* If we did find the corresponding PHI, copy those inputs.  */
>       if (ophi)
>        {
> +         /* If NOP is used somewhere else beyond nphi, give up.  */
> +         if (!has_single_use (nop))
> +           goto fail;
>          bitmap_set_bit (ophi_handled, SSA_NAME_VERSION (nop));
>          FOR_EACH_EDGE (e, ei, old_bb->preds)
>            {
> --- gcc/testsuite/g++.dg/opt/pr47355.C.jj       2011-01-20 11:29:54.000000000 +0100
> +++ gcc/testsuite/g++.dg/opt/pr47355.C  2011-01-20 11:29:29.000000000 +0100
> @@ -0,0 +1,39 @@
> +// PR tree-optimization/47355
> +// { dg-do compile }
> +// { dg-options "-O -fipa-cp -fipa-cp-clone" }
> +
> +struct T
> +{
> +  T ();
> +  void *p;
> +  ~T ();
> +};
> +
> +void foo (T *i);
> +
> +T *bar ();
> +void baz (T *);
> +
> +struct V
> +{
> +  long q;
> +  T *r;
> +  ~V ()
> +  {
> +    while (q)
> +      {
> +       foo (r);
> +       ++r;
> +       --q;
> +      }
> +    baz (r);
> +  }
> +};
> +
> +void
> +foo ()
> +{
> +  V v;
> +  T t;
> +  v.r = bar ();
> +}
>
>        Jakub
>

Patch

--- gcc/tree-eh.c.jj	2011-01-18 18:16:11.000000000 +0100
+++ gcc/tree-eh.c	2011-01-20 11:11:48.000000000 +0100
@@ -3552,6 +3552,9 @@  cleanup_empty_eh_merge_phis (basic_block
       /* If we did find the corresponding PHI, copy those inputs.  */
       if (ophi)
 	{
+	  /* If NOP is used somewhere else beyond nphi, give up.  */
+	  if (!has_single_use (nop))
+	    goto fail;
 	  bitmap_set_bit (ophi_handled, SSA_NAME_VERSION (nop));
 	  FOR_EACH_EDGE (e, ei, old_bb->preds)
 	    {
--- gcc/testsuite/g++.dg/opt/pr47355.C.jj	2011-01-20 11:29:54.000000000 +0100
+++ gcc/testsuite/g++.dg/opt/pr47355.C	2011-01-20 11:29:29.000000000 +0100
@@ -0,0 +1,39 @@ 
+// PR tree-optimization/47355
+// { dg-do compile }
+// { dg-options "-O -fipa-cp -fipa-cp-clone" }
+
+struct T
+{
+  T ();
+  void *p;
+  ~T ();
+};
+
+void foo (T *i);
+
+T *bar ();
+void baz (T *);
+
+struct V
+{
+  long q;
+  T *r;
+  ~V ()
+  {
+    while (q)
+      {
+	foo (r);
+	++r;
+	--q;
+      }
+    baz (r);
+  }
+};
+
+void
+foo ()
+{
+  V v;
+  T t;
+  v.r = bar ();
+}