Re: [PATCH] Fix devirtualization ICE (PR tree-optimization/59622, take 3)

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Jan 09, 2014 at 09:46:11AM +0100, Richard Biener wrote:
> > +	      /* If fndecl (like __builtin_unreachable or
> > +		 __cxa_pure_virtual) takes no arguments, doesn't have
> > +		 return value and is noreturn, if the call doesn't have
> > +		 lhs or lhs isn't SSA_NAME, replace the call with
> > +		 the noreturn call, otherwise insert it before the call
> > +		 and replace the call with setting of lhs to default def.  */
> > +	      if (TREE_THIS_VOLATILE (fndecl)
> > +		  && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (fndecl)))
> > +		  && TYPE_ARG_TYPES (TREE_TYPE (fndecl)) == void_list_node)
> 
> ... restrict this to the targets.length () == 0 case.

Well, then the __cxa_pure_virtual testcases ICE again, but the pr59622-5.C
testcase ICEs anyway, so here is a different patch (untested so far except
for the tests).  The issue with the calls is that when fold_stmt is done
during gimplification (or omp lowering), we don't have cfg nor SSA form and
nothing performs fixup_noreturn_calls (that function requires CFG and SSA
form anyway).  So, we have to fix that up by hand, but as we aren't in SSA
form yet, dropping lhs is always safe for the noreturn calls.

2014-01-09  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/59622
	* gimple-fold.c (gimple_fold_call): Fix a typo in message.  For
	__builtin_unreachable replace the OBJ_TYPE_REF call with a call to
	__builtin_unreachable and add if needed a setter of the lhs SSA_NAME.
	Don't devirtualize for inplace at all.  For targets.length () == 1,
	if the call is noreturn and cfun isn't in SSA form yet, clear lhs.

	* g++.dg/opt/pr59622-2.C: New test.
	* g++.dg/opt/pr59622-3.C: New test.
	* g++.dg/opt/pr59622-4.C: New test.
	* g++.dg/opt/pr59622-5.C: New test.

--- gcc/gimple-fold.c.jj	2014-01-08 17:44:57.690582374 +0100
+++ gcc/gimple-fold.c	2014-01-09 11:05:40.165287975 +0100
@@ -1167,7 +1167,7 @@ gimple_fold_call (gimple_stmt_iterator *
                                                  (OBJ_TYPE_REF_EXPR (callee)))))
 	    {
 	      fprintf (dump_file,
-		       "Type inheritnace inconsistent devirtualization of ");
+		       "Type inheritance inconsistent devirtualization of ");
 	      print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
 	      fprintf (dump_file, " to ");
 	      print_generic_expr (dump_file, callee, TDF_SLIM);
@@ -1177,7 +1177,7 @@ gimple_fold_call (gimple_stmt_iterator *
 	  gimple_call_set_fn (stmt, OBJ_TYPE_REF_EXPR (callee));
 	  changed = true;
 	}
-      else if (flag_devirtualize && virtual_method_call_p (callee))
+      else if (flag_devirtualize && !inplace && virtual_method_call_p (callee))
 	{
 	  bool final;
 	  vec <cgraph_node *>targets
@@ -1188,13 +1188,29 @@ gimple_fold_call (gimple_stmt_iterator *
 		{
 		  gimple_call_set_fndecl (stmt, targets[0]->decl);
 		  changed = true;
+		  /* If the call becomes noreturn and we aren't in SSA form
+		     yet, just drop the lhs, because fixup_noreturn_call
+		     isn't run at that point yet.  */
+		  if ((gimple_call_flags (stmt) & ECF_NORETURN)
+		      && !gimple_in_ssa_p (cfun)
+		      && gimple_call_lhs (stmt))
+		    gimple_call_set_lhs (stmt, NULL_TREE);
 		}
-	      else if (!inplace)
+	      else
 		{
+		  tree lhs = gimple_call_lhs (stmt);
 		  tree fndecl = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
 		  gimple new_stmt = gimple_build_call (fndecl, 0);
 		  gimple_set_location (new_stmt, gimple_location (stmt));
-		  gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
+		  if (lhs && TREE_CODE (lhs) == SSA_NAME)
+		    {
+		      tree var = create_tmp_var (TREE_TYPE (lhs), NULL);
+		      tree def = get_or_create_ssa_default_def (cfun, var);
+		      gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
+		      update_call_from_tree (gsi, def);
+		    }
+		  else
+		    gsi_replace (gsi, new_stmt, true);
 		  return true;
 		}
 	    }
--- gcc/testsuite/g++.dg/opt/pr59622-2.C.jj	2014-01-09 10:57:46.246694025 +0100
+++ gcc/testsuite/g++.dg/opt/pr59622-2.C	2014-01-09 10:57:46.246694025 +0100
@@ -0,0 +1,21 @@
+// PR tree-optimization/59622
+// { dg-do compile }
+// { dg-options "-O2" }
+
+namespace
+{
+  struct A
+  {
+    A () {}
+    virtual A *bar (int) = 0;
+    A *baz (int x) { return bar (x); }
+  };
+}
+
+A *a;
+
+void
+foo ()
+{
+  a->baz (0);
+}
--- gcc/testsuite/g++.dg/opt/pr59622-3.C.jj	2014-01-09 10:57:46.247694040 +0100
+++ gcc/testsuite/g++.dg/opt/pr59622-3.C	2014-01-09 10:57:46.247694040 +0100
@@ -0,0 +1,21 @@
+// PR tree-optimization/59622
+// { dg-do compile }
+// { dg-options "-O2" }
+
+struct C { int a; int b; };
+
+namespace
+{
+  struct A
+  {
+    virtual C foo ();
+    C bar () { return foo (); }
+  };
+}
+
+C
+baz ()
+{
+  A a;
+  return a.bar ();
+}
--- gcc/testsuite/g++.dg/opt/pr59622-4.C.jj	2014-01-09 10:57:46.247694040 +0100
+++ gcc/testsuite/g++.dg/opt/pr59622-4.C	2014-01-09 10:57:46.247694040 +0100
@@ -0,0 +1,23 @@
+// PR tree-optimization/59622
+// { dg-do compile }
+// { dg-options "-O2" }
+
+struct C { int a; int b; };
+
+namespace
+{
+  struct A
+  {
+    A () {}
+    virtual C bar (int) = 0;
+    C baz (int x) { return bar (x); }
+  };
+}
+
+A *a;
+
+C
+foo ()
+{
+  return a->baz (0);
+}
--- gcc/testsuite/g++.dg/opt/pr59622-5.C.jj	2014-01-09 11:06:11.862133398 +0100
+++ gcc/testsuite/g++.dg/opt/pr59622-5.C	2014-01-09 10:54:04.000000000 +0100
@@ -0,0 +1,26 @@
+// PR tree-optimization/59622
+// { dg-do compile }
+// { dg-options "-O2" }
+
+namespace
+{
+  struct A
+  {
+    A () {}
+    virtual A *bar (int);
+    A *baz (int x) { return bar (x); }
+  };
+
+  __attribute__((noreturn)) A *A::bar (int)
+  {
+    __builtin_exit (0);
+  }
+}
+
+A *a;
+
+void
+foo ()
+{
+  a->baz (0);
+}


	Jakub