public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR68470, ICE in IPA split
@ 2015-11-27  9:36 Richard Biener
  2015-11-27 11:27 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Richard Biener @ 2015-11-27  9:36 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka


This fixes the case where we split the returning part of a function
and keep the non-returning part as main.  We were keeping the return
block with stmts not relevant to main in the return path of the
split which obviously doesn't work as it may use SSA names no longer
defined (but split out).

The following patch detects the situation and pretends the exit
block was found as EXIT_BLOCK_FOR_FN in this case.

Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu.

Honza, does this look ok?

Thanks,
Richard.

2015-11-27  Richard Biener  <rguenther@suse.de>

	PR ipa/68470
	* ipa-split.c (split_function): Handle main part not returning.

	* g++.dg/torture/pr68470.C: New testcase.

Index: gcc/ipa-split.c
===================================================================
--- gcc/ipa-split.c	(revision 230998)
+++ gcc/ipa-split.c	(working copy)
@@ -1205,7 +1205,6 @@ split_function (basic_block return_bb, s
   edge e;
   edge_iterator ei;
   tree retval = NULL, real_retval = NULL, retbnd = NULL;
-  bool split_part_return_p = false;
   bool with_bounds = chkp_function_instrumented_p (current_function_decl);
   gimple *last_stmt = NULL;
   unsigned int i;
@@ -1246,12 +1245,16 @@ split_function (basic_block return_bb, s
 	args_to_pass.safe_push (arg);
       }
 
-  /* See if the split function will return.  */
+  /* See if the split function or the main part will return.  */
+  bool main_part_return_p = false;
+  bool split_part_return_p = false;
   FOR_EACH_EDGE (e, ei, return_bb->preds)
-    if (bitmap_bit_p (split_point->split_bbs, e->src->index))
-      break;
-  if (e)
-    split_part_return_p = true;
+    {
+      if (bitmap_bit_p (split_point->split_bbs, e->src->index))
+	split_part_return_p = true;
+      else
+	main_part_return_p = true;
+    }
 
   /* Add return block to what will become the split function.
      We do not return; no return block is needed.  */
@@ -1295,6 +1298,11 @@ split_function (basic_block return_bb, s
   else
     bitmap_set_bit (split_point->split_bbs, return_bb->index);
 
+  /* If the main part doesn't return pretend the return block wasn't
+     found for all of the following.  */
+  if (! main_part_return_p)
+    return_bb = EXIT_BLOCK_PTR_FOR_FN (cfun);
+
   /* If RETURN_BB has virtual operand PHIs, they must be removed and the
      virtual operand marked for renaming as we change the CFG in a way that
      tree-inline is not able to compensate for.
Index: gcc/testsuite/g++.dg/torture/pr68470.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr68470.C	(revision 0)
+++ gcc/testsuite/g++.dg/torture/pr68470.C	(working copy)
@@ -0,0 +1,36 @@
+/* { dg-do compile } */
+
+void deallocate(void *);
+void *a;
+
+struct C {
+    virtual void m_fn1();
+};
+
+struct D {
+    C *m_fn2() {
+	if (a)
+	  __builtin_abort();
+    }
+};
+D getd();
+
+struct vec_int {
+    int _M_start;
+    ~vec_int() {
+	if (_M_start)
+	  deallocate(&_M_start);
+    }
+};
+vec_int *b;
+
+struct I {
+    virtual void m_fn3();
+};
+
+void I::m_fn3() {
+    if (a)
+      getd().m_fn2()->m_fn1();
+    b->~vec_int();
+}
+

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

* Re: [PATCH] Fix PR68470, ICE in IPA split
  2015-11-27  9:36 [PATCH] Fix PR68470, ICE in IPA split Richard Biener
@ 2015-11-27 11:27 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2015-11-27 11:27 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka

On Fri, 27 Nov 2015, Richard Biener wrote:

> 
> This fixes the case where we split the returning part of a function
> and keep the non-returning part as main.  We were keeping the return
> block with stmts not relevant to main in the return path of the
> split which obviously doesn't work as it may use SSA names no longer
> defined (but split out).
> 
> The following patch detects the situation and pretends the exit
> block was found as EXIT_BLOCK_FOR_FN in this case.
> 
> Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu.
> 
> Honza, does this look ok?

Ok, fails for example

FAIL: gcc.dg/vect/pr51581-1.c execution test

where we split a region at a point the main part falls thru to
(an odd place to split...).  Happens when we split at a loop
header but the pre-header remains in the main part.

Adjusted patch as follows.

Richard.

2015-11-27  Richard Biener  <rguenther@suse.de>

	PR ipa/68470
	* ipa-split.c (split_function): Handle main part not returning.

	* g++.dg/torture/pr68470.C: New testcase.

Index: gcc/ipa-split.c
===================================================================
--- gcc/ipa-split.c	(revision 230998)
+++ gcc/ipa-split.c	(working copy)
@@ -1205,7 +1205,6 @@ split_function (basic_block return_bb, s
   edge e;
   edge_iterator ei;
   tree retval = NULL, real_retval = NULL, retbnd = NULL;
-  bool split_part_return_p = false;
   bool with_bounds = chkp_function_instrumented_p (current_function_decl);
   gimple *last_stmt = NULL;
   unsigned int i;
@@ -1246,12 +1245,28 @@ split_function (basic_block return_bb, s
 	args_to_pass.safe_push (arg);
       }
 
-  /* See if the split function will return.  */
+  /* See if the split function or the main part will return.  */
+  bool main_part_return_p = false;
+  bool split_part_return_p = false;
   FOR_EACH_EDGE (e, ei, return_bb->preds)
-    if (bitmap_bit_p (split_point->split_bbs, e->src->index))
-      break;
-  if (e)
-    split_part_return_p = true;
+    {
+      if (bitmap_bit_p (split_point->split_bbs, e->src->index))
+	split_part_return_p = true;
+      else
+	main_part_return_p = true;
+    }
+  /* The main part also returns if we we split on a fallthru edge
+     and the split part returns.  */
+  if (split_part_return_p)
+    FOR_EACH_EDGE (e, ei, split_point->entry_bb->preds)
+      {
+	if (! bitmap_bit_p (split_point->split_bbs, e->src->index)
+	    && single_succ_p (e->src))
+	  {
+	    main_part_return_p = true;
+	    break;
+	  }
+      }
 
   /* Add return block to what will become the split function.
      We do not return; no return block is needed.  */
@@ -1295,6 +1310,11 @@ split_function (basic_block return_bb, s
   else
     bitmap_set_bit (split_point->split_bbs, return_bb->index);
 
+  /* If the main part doesn't return pretend the return block wasn't
+     found for all of the following.  */
+  if (! main_part_return_p)
+    return_bb = EXIT_BLOCK_PTR_FOR_FN (cfun);
+
   /* If RETURN_BB has virtual operand PHIs, they must be removed and the
      virtual operand marked for renaming as we change the CFG in a way that
      tree-inline is not able to compensate for.
Index: gcc/testsuite/g++.dg/torture/pr68470.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr68470.C	(revision 0)
+++ gcc/testsuite/g++.dg/torture/pr68470.C	(working copy)
@@ -0,0 +1,36 @@
+/* { dg-do compile } */
+
+void deallocate(void *);
+void *a;
+
+struct C {
+    virtual void m_fn1();
+};
+
+struct D {
+    C *m_fn2() {
+	if (a)
+	  __builtin_abort();
+    }
+};
+D getd();
+
+struct vec_int {
+    int _M_start;
+    ~vec_int() {
+	if (_M_start)
+	  deallocate(&_M_start);
+    }
+};
+vec_int *b;
+
+struct I {
+    virtual void m_fn3();
+};
+
+void I::m_fn3() {
+    if (a)
+      getd().m_fn2()->m_fn1();
+    b->~vec_int();
+}
+

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

end of thread, other threads:[~2015-11-27 11:10 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-27  9:36 [PATCH] Fix PR68470, ICE in IPA split Richard Biener
2015-11-27 11:27 ` Richard Biener

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).