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

login
register
mail settings
Submitter Jakub Jelinek
Date Jan. 21, 2011, 12:37 p.m.
Message ID <20110121123700.GE2724@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/79834/
State New
Headers show

Comments

Jakub Jelinek - Jan. 21, 2011, 12:37 p.m.
On Fri, Jan 21, 2011 at 11:26:32AM +0100, Richard Guenther wrote:
> On Thu, Jan 20, 2011 at 5:29 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > 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.

Actually, I've run another bootstrap/regtests with gathering statistics
and there were ~100 cases in which the !has_single_use test refused to move
something that was actually movable.  In particular if nop was used by more
than one PHI node on new_bb.  cleanup_empty_eh_merge_phis handles that
perfectly, so this patch allows those cases too.  With this patch
the only cases rejected by cleanup_empty_eh_merge_phis during x86_64-linux
and i686-linux bootstrap/regtests is the newly added testcase pr47355.C.

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

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

	PR tree-optimization/47355
	* tree-eh.c (cleanup_empty_eh_merge_phis): Give up if
	NOP has non-debug uses beyond PHIs in new_bb.

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


	Jakub
Richard Guenther - Jan. 21, 2011, 12:47 p.m.
On Fri, Jan 21, 2011 at 1:37 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Jan 21, 2011 at 11:26:32AM +0100, Richard Guenther wrote:
>> On Thu, Jan 20, 2011 at 5:29 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > 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.
>
> Actually, I've run another bootstrap/regtests with gathering statistics
> and there were ~100 cases in which the !has_single_use test refused to move
> something that was actually movable.  In particular if nop was used by more
> than one PHI node on new_bb.  cleanup_empty_eh_merge_phis handles that
> perfectly, so this patch allows those cases too.  With this patch
> the only cases rejected by cleanup_empty_eh_merge_phis during x86_64-linux
> and i686-linux bootstrap/regtests is the newly added testcase pr47355.C.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2011-01-21  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/47355
>        * tree-eh.c (cleanup_empty_eh_merge_phis): Give up if
>        NOP has non-debug uses beyond PHIs in new_bb.
>
>        * 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-21 13:22:41.000000000 +0100
> @@ -3552,6 +3552,20 @@ 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 phis in new_bb, give up.  */
> +         if (!has_single_use (nop))
> +           {
> +             imm_use_iterator imm_iter;
> +             use_operand_p use_p;
> +
> +             FOR_EACH_IMM_USE_FAST (use_p, imm_iter, nop)
> +               {
> +                 if (!gimple_debug_bind_p (USE_STMT (use_p))
> +                     && (gimple_code (USE_STMT (use_p)) != GIMPLE_PHI
> +                         || gimple_bb (USE_STMT (use_p)) != new_bb))
> +                   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-21 13:22:41.000000000 +0100
@@ -3552,6 +3552,20 @@  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 phis in new_bb, give up.  */
+	  if (!has_single_use (nop))
+	    {
+	      imm_use_iterator imm_iter;
+	      use_operand_p use_p;
+
+	      FOR_EACH_IMM_USE_FAST (use_p, imm_iter, nop)
+		{
+		  if (!gimple_debug_bind_p (USE_STMT (use_p))
+		      && (gimple_code (USE_STMT (use_p)) != GIMPLE_PHI
+			  || gimple_bb (USE_STMT (use_p)) != new_bb))
+		    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 ();
+}