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

Richard Biener rguenther@suse.de
Thu Jan 9 11:15:00 GMT 2014


On Thu, 9 Jan 2014, Jakub Jelinek wrote:

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

Don't we "fix this up" during CFG build?  That is, I fail to see the
issue with noreturn and pre-CFG?

Richard.

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

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer



More information about the Gcc-patches mailing list