Patchwork Fix PR ipa/60315 (inliner explosion)

login
register
mail settings
Submitter Jan Hubicka
Date March 27, 2014, 7:11 p.m.
Message ID <20140327191118.GA24858@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/334454/
State New
Headers show

Comments

Jan Hubicka - March 27, 2014, 7:11 p.m.
> > Bootstrapped/regtested x86_64-linux, comitted.
> 
> Not with Ada apparently, resulting in 
> 
>                 === acats tests ===
> FAIL:   c34007d
> FAIL:   c34007g
> FAIL:   c34007s
> FAIL:   c37213j
> FAIL:   c37213k
> FAIL:   c37213l
> FAIL:   ce2201g
> FAIL:   cxa5a03
> FAIL:   cxa5a04
> FAIL:   cxa5a06
> FAIL:   cxg2013
> FAIL:   cxg2015
> 
The problem is that by redirection to noreturn, we end up freeing SSA name of the
LHS but later we still process statements that refer it until they are removed as
unreachable.
The following patch fixes it. I tested it on x86_64-linux, but changed my mind.
I think fixup_noreturn_call should do it instead, I will send updated patch after
testing.

Honza
Jan Hubicka - March 28, 2014, 3:32 p.m.
> > > Bootstrapped/regtested x86_64-linux, comitted.
> > 
> > Not with Ada apparently, resulting in 
> > 
> >                 === acats tests ===
> > FAIL:   c34007d
> > FAIL:   c34007g
> > FAIL:   c34007s
> > FAIL:   c37213j
> > FAIL:   c37213k
> > FAIL:   c37213l
> > FAIL:   ce2201g
> > FAIL:   cxa5a03
> > FAIL:   cxa5a04
> > FAIL:   cxa5a06
> > FAIL:   cxg2013
> > FAIL:   cxg2015
> > 
> The problem is that by redirection to noreturn, we end up freeing SSA name of the
> LHS but later we still process statements that refer it until they are removed as
> unreachable.
> The following patch fixes it. I tested it on x86_64-linux, but changed my mind.
> I think fixup_noreturn_call should do it instead, I will send updated patch after
> testing.
> 
> Honza

Actually after some additional invetstigation I decided to commit this patch.
fixup_noreturn_call already cares about the return value but differently than the
new Jakub's code.  We ought to unify it, but only for next stage1. 

Honza
> 
> Index: cgraph.c
> ===================================================================
> --- cgraph.c	(revision 208875)
> +++ cgraph.c	(working copy)
> @@ -1329,6 +1331,7 @@ gimple
>  cgraph_redirect_edge_call_stmt_to_callee (struct cgraph_edge *e)
>  {
>    tree decl = gimple_call_fndecl (e->call_stmt);
> +  tree lhs = gimple_call_lhs (e->call_stmt);
>    gimple new_stmt;
>    gimple_stmt_iterator gsi;
>  #ifdef ENABLE_CHECKING
> @@ -1471,6 +1474,22 @@ cgraph_redirect_edge_call_stmt_to_callee
>        update_stmt_fn (DECL_STRUCT_FUNCTION (e->caller->decl), new_stmt);
>      }
>  
> +  /* If the call becomes noreturn, remove the lhs.  */
> +  if (lhs && (gimple_call_flags (new_stmt) & ECF_NORETURN))
> +    {
> +      if (TREE_CODE (lhs) == SSA_NAME)
> +	{
> +          gsi = gsi_for_stmt (new_stmt);
> +
> +	  tree var = create_tmp_var (TREE_TYPE (lhs), NULL);
> +	  tree def = get_or_create_ssa_default_def
> +		      (DECL_STRUCT_FUNCTION (e->caller->decl), var);
> +	  gimple set_stmt = gimple_build_assign (lhs, def);
> +	  gsi_insert_before (&gsi, set_stmt, GSI_SAME_STMT);
> +	}
> +      gimple_call_set_lhs (new_stmt, NULL_TREE);
> +    }
> +
>    cgraph_set_call_stmt_including_clones (e->caller, e->call_stmt, new_stmt, false);
>  
>    if (cgraph_dump_file)
Eric Botcazou - March 28, 2014, 6:12 p.m.
> Actually after some additional invetstigation I decided to commit this
> patch. fixup_noreturn_call already cares about the return value but
> differently than the new Jakub's code. 

Thanks for the quick fix, I confirm that the ACATS failures are all gone.

So we're left with the GIMPLE checking failure on opt33.adb.

Patch

Index: cgraph.c
===================================================================
--- cgraph.c	(revision 208875)
+++ cgraph.c	(working copy)
@@ -1329,6 +1331,7 @@  gimple
 cgraph_redirect_edge_call_stmt_to_callee (struct cgraph_edge *e)
 {
   tree decl = gimple_call_fndecl (e->call_stmt);
+  tree lhs = gimple_call_lhs (e->call_stmt);
   gimple new_stmt;
   gimple_stmt_iterator gsi;
 #ifdef ENABLE_CHECKING
@@ -1471,6 +1474,22 @@  cgraph_redirect_edge_call_stmt_to_callee
       update_stmt_fn (DECL_STRUCT_FUNCTION (e->caller->decl), new_stmt);
     }
 
+  /* If the call becomes noreturn, remove the lhs.  */
+  if (lhs && (gimple_call_flags (new_stmt) & ECF_NORETURN))
+    {
+      if (TREE_CODE (lhs) == SSA_NAME)
+	{
+          gsi = gsi_for_stmt (new_stmt);
+
+	  tree var = create_tmp_var (TREE_TYPE (lhs), NULL);
+	  tree def = get_or_create_ssa_default_def
+		      (DECL_STRUCT_FUNCTION (e->caller->decl), var);
+	  gimple set_stmt = gimple_build_assign (lhs, def);
+	  gsi_insert_before (&gsi, set_stmt, GSI_SAME_STMT);
+	}
+      gimple_call_set_lhs (new_stmt, NULL_TREE);
+    }
+
   cgraph_set_call_stmt_including_clones (e->caller, e->call_stmt, new_stmt, false);
 
   if (cgraph_dump_file)