Patchwork Fix ICE with MEM_REF clobbers with anonymous SSA_NAME (PR tree-optimization/57661)

login
register
mail settings
Submitter Jakub Jelinek
Date Aug. 13, 2013, 9:59 a.m.
Message ID <20130813095912.GD1814@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/266764/
State New
Headers show

Comments

Jakub Jelinek - Aug. 13, 2013, 9:59 a.m.
Hi!

On the following testcase we ICE because we have an indirect (MEM_REF)
clobber stmt that uses anonymous SSA_NAME.  ipa-split.c (intentionally)
ignores clobber stmts because they don't result in executable code,
but that can lead into clobber stmts mentioning SSA_NAMEs that aren't
initialized.  For non-anonymous SSA_NAMEs that is not an issue, we just drop
those during tree-ssa-live.c later on (clobber stmts are always optional
and furthermore in ipa-split case the clobber stmts are duplicated both
in the (to be inlined) caller and callee), but for anonymous SSA_NAMEs
there is no such thing as (D) and thus we ICE.

Fixed by removing those clobber stmts already during function versioning
during ipa-split.c.

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

2013-08-13  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/57661
	* tree-inline.h (struct copy_body_data): Add blocks_to_copy field.
	* tree-inline.c (tree_function_versioning): Initialize it.
	(remap_gimple_stmt): Return GIMPLE_NOP for MEM_REF lhs clobber stmts
	if id->blocks_to_copy and MEM_REF's SSA_NAME is defined in a block
	that is not being copied.

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


	Jakub
Richard Guenther - Aug. 13, 2013, 3:21 p.m.
Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>On the following testcase we ICE because we have an indirect (MEM_REF)
>clobber stmt that uses anonymous SSA_NAME.  ipa-split.c (intentionally)
>ignores clobber stmts because they don't result in executable code,
>but that can lead into clobber stmts mentioning SSA_NAMEs that aren't
>initialized.  For non-anonymous SSA_NAMEs that is not an issue, we just
>drop
>those during tree-ssa-live.c later on (clobber stmts are always
>optional
>and furthermore in ipa-split case the clobber stmts are duplicated both
>in the (to be inlined) caller and callee), but for anonymous SSA_NAMEs
>there is no such thing as (D) and thus we ICE.
>
>Fixed by removing those clobber stmts already during function
>versioning
>during ipa-split.c.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
Ok.

Thanks,
Richard.

>2013-08-13  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/57661
>	* tree-inline.h (struct copy_body_data): Add blocks_to_copy field.
>	* tree-inline.c (tree_function_versioning): Initialize it.
>	(remap_gimple_stmt): Return GIMPLE_NOP for MEM_REF lhs clobber stmts
>	if id->blocks_to_copy and MEM_REF's SSA_NAME is defined in a block
>	that is not being copied.
>
>	* g++.dg/opt/pr57661.C: New test.
>
>--- gcc/tree-inline.h.jj	2013-06-03 18:11:57.000000000 +0200
>+++ gcc/tree-inline.h	2013-07-23 00:38:14.362153447 +0200
>@@ -115,6 +115,10 @@ typedef struct copy_body_data
>   /* Entry basic block to currently copied body.  */
>   basic_block entry_bb;
> 
>+  /* For partial function versioning, bitmap of bbs to be copied,
>+     otherwise NULL.  */
>+  bitmap blocks_to_copy;
>+
>   /* Debug statements that need processing.  */
>   vec<gimple> debug_stmts;
> 
>--- gcc/tree-inline.c.jj	2013-07-06 09:24:10.000000000 +0200
>+++ gcc/tree-inline.c	2013-07-23 01:30:39.575893735 +0200
>@@ -1387,6 +1387,23 @@ remap_gimple_stmt (gimple stmt, copy_bod
> 	    }
> 	}
> 
>+      /* For *ptr_N ={v} {CLOBBER}, if ptr_N is SSA_NAME defined
>+	 in a block that we aren't copying during tree_function_versioning,
>+	 just drop the clobber stmt.  */
>+      if (id->blocks_to_copy && gimple_clobber_p (stmt))
>+	{
>+	  tree lhs = gimple_assign_lhs (stmt);
>+	  if (TREE_CODE (lhs) == MEM_REF
>+	      && TREE_CODE (TREE_OPERAND (lhs, 0)) == SSA_NAME)
>+	    {
>+	      gimple def_stmt = SSA_NAME_DEF_STMT (TREE_OPERAND (lhs, 0));
>+	      if (gimple_bb (def_stmt)
>+		  && !bitmap_bit_p (id->blocks_to_copy,
>+				    gimple_bb (def_stmt)->index))
>+		return gimple_build_nop ();
>+	    }
>+	}
>+
>       if (gimple_debug_bind_p (stmt))
> 	{
> 	  copy = gimple_build_debug_bind (gimple_debug_bind_get_var (stmt),
>@@ -5091,6 +5108,7 @@ tree_function_versioning (tree old_decl,
>   id.src_node = old_version_node;
>   id.dst_node = new_version_node;
>   id.src_cfun = DECL_STRUCT_FUNCTION (old_decl);
>+  id.blocks_to_copy = blocks_to_copy;
>   if (id.src_node->ipa_transforms_to_apply.exists ())
>     {
>       vec<ipa_opt_pass> old_transforms_to_apply
>--- gcc/testsuite/g++.dg/opt/pr57661.C.jj	2013-07-23 01:42:45.078284916
>+0200
>+++ gcc/testsuite/g++.dg/opt/pr57661.C	2013-07-23 01:41:41.000000000
>+0200
>@@ -0,0 +1,76 @@
>+// PR tree-optimization/57661
>+// { dg-do compile }
>+// { dg-options "-O2 -fno-tree-forwprop -std=gnu++11" }
>+
>+template <typename>
>+struct A
>+{
>+  ~A () {}
>+};
>+template <typename _Tp>
>+using B = A <_Tp>;
>+template <typename _Tp>
>+class C : B <_Tp> {};
>+namespace N { enum D { d }; }
>+template <class>
>+struct E
>+{
>+  ~E ();
>+};
>+template <class, class V>
>+struct F : V {};
>+template <class U, class V>
>+struct G : F <U, V>
>+{
>+  N::D g1;
>+  void g2 ();
>+  void g3 ();
>+  void g4 () { g3 (); }
>+  static void g5 (G *__t) { __t->g4 (); }
>+};
>+template <class U, class V>
>+struct H : G <U, V>
>+{
>+  E <U> *h1;
>+  bool h2;
>+  ~H () throw ()
>+  {
>+    this->g2 ();
>+    if (h2)
>+      delete h1;
>+  }
>+};
>+template <class U, class V>
>+struct I : H <U, V>, E <U>
>+{
>+  G <U, V> *i;
>+  ~I () throw ()
>+  {
>+    i->g4 ();
>+  }
>+};
>+struct J
>+{
>+  typedef C <char> j1;
>+  typedef G <char, C <char>> j2;
>+  J ();
>+  j2 *j3;
>+};
>+struct K : J
>+{
>+  typedef G <char, C <char>> j2;
>+  K () { j2::g5 (this->j3); }
>+};
>+template <class U, class V>
>+void G <U, V>::g3 ()
>+{
>+  switch (g1)
>+    {
>+    case N::d:
>+      {
>+	I <U, V> *q = (I <U, V> *) this;
>+	q->I <U, V>::~I ();
>+      }
>+    }
>+}
>+K r;
>
>	Jakub

Patch

--- gcc/tree-inline.h.jj	2013-06-03 18:11:57.000000000 +0200
+++ gcc/tree-inline.h	2013-07-23 00:38:14.362153447 +0200
@@ -115,6 +115,10 @@  typedef struct copy_body_data
   /* Entry basic block to currently copied body.  */
   basic_block entry_bb;
 
+  /* For partial function versioning, bitmap of bbs to be copied,
+     otherwise NULL.  */
+  bitmap blocks_to_copy;
+
   /* Debug statements that need processing.  */
   vec<gimple> debug_stmts;
 
--- gcc/tree-inline.c.jj	2013-07-06 09:24:10.000000000 +0200
+++ gcc/tree-inline.c	2013-07-23 01:30:39.575893735 +0200
@@ -1387,6 +1387,23 @@  remap_gimple_stmt (gimple stmt, copy_bod
 	    }
 	}
 
+      /* For *ptr_N ={v} {CLOBBER}, if ptr_N is SSA_NAME defined
+	 in a block that we aren't copying during tree_function_versioning,
+	 just drop the clobber stmt.  */
+      if (id->blocks_to_copy && gimple_clobber_p (stmt))
+	{
+	  tree lhs = gimple_assign_lhs (stmt);
+	  if (TREE_CODE (lhs) == MEM_REF
+	      && TREE_CODE (TREE_OPERAND (lhs, 0)) == SSA_NAME)
+	    {
+	      gimple def_stmt = SSA_NAME_DEF_STMT (TREE_OPERAND (lhs, 0));
+	      if (gimple_bb (def_stmt)
+		  && !bitmap_bit_p (id->blocks_to_copy,
+				    gimple_bb (def_stmt)->index))
+		return gimple_build_nop ();
+	    }
+	}
+
       if (gimple_debug_bind_p (stmt))
 	{
 	  copy = gimple_build_debug_bind (gimple_debug_bind_get_var (stmt),
@@ -5091,6 +5108,7 @@  tree_function_versioning (tree old_decl,
   id.src_node = old_version_node;
   id.dst_node = new_version_node;
   id.src_cfun = DECL_STRUCT_FUNCTION (old_decl);
+  id.blocks_to_copy = blocks_to_copy;
   if (id.src_node->ipa_transforms_to_apply.exists ())
     {
       vec<ipa_opt_pass> old_transforms_to_apply
--- gcc/testsuite/g++.dg/opt/pr57661.C.jj	2013-07-23 01:42:45.078284916 +0200
+++ gcc/testsuite/g++.dg/opt/pr57661.C	2013-07-23 01:41:41.000000000 +0200
@@ -0,0 +1,76 @@ 
+// PR tree-optimization/57661
+// { dg-do compile }
+// { dg-options "-O2 -fno-tree-forwprop -std=gnu++11" }
+
+template <typename>
+struct A
+{
+  ~A () {}
+};
+template <typename _Tp>
+using B = A <_Tp>;
+template <typename _Tp>
+class C : B <_Tp> {};
+namespace N { enum D { d }; }
+template <class>
+struct E
+{
+  ~E ();
+};
+template <class, class V>
+struct F : V {};
+template <class U, class V>
+struct G : F <U, V>
+{
+  N::D g1;
+  void g2 ();
+  void g3 ();
+  void g4 () { g3 (); }
+  static void g5 (G *__t) { __t->g4 (); }
+};
+template <class U, class V>
+struct H : G <U, V>
+{
+  E <U> *h1;
+  bool h2;
+  ~H () throw ()
+  {
+    this->g2 ();
+    if (h2)
+      delete h1;
+  }
+};
+template <class U, class V>
+struct I : H <U, V>, E <U>
+{
+  G <U, V> *i;
+  ~I () throw ()
+  {
+    i->g4 ();
+  }
+};
+struct J
+{
+  typedef C <char> j1;
+  typedef G <char, C <char>> j2;
+  J ();
+  j2 *j3;
+};
+struct K : J
+{
+  typedef G <char, C <char>> j2;
+  K () { j2::g5 (this->j3); }
+};
+template <class U, class V>
+void G <U, V>::g3 ()
+{
+  switch (g1)
+    {
+    case N::d:
+      {
+	I <U, V> *q = (I <U, V> *) this;
+	q->I <U, V>::~I ();
+      }
+    }
+}
+K r;