[PATCH] Fix -fcompare-debug failure during inlining (PR tree-optimization/44182)

[Richard Guenther]

On Mon, 31 May 2010, Jakub Jelinek wrote:

> Hi!
> 
> On the below included testcase a call isn't stmt_can_throw_internal
> before inlining (which means it doesn't need to end a basic block).
> Statements after it in the same bb get optimized away with -g0,
> but some debug stmts stay there with -g.  Then when the function
> containing this call is inlined the call suddenly gets an EH region and
> so is stmt_can_throw_internal.  For -g0 the call already ends a bb,
> but with -g it doesn't and copy_edges_for_bb splits the bb, which leads to
> various differences in generated code.
> 
> The following patch fixes this by copying the debug stmts to the successors,
> either with the same value (if the successors have just a single
> predecessor) or resetting its value and then dropping the original debug
> stmts.  During copy_edges_for_bb the edges aren't finalized yet, so this
> copying/removal is deferred until the edges are finalized.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux.  Ok for trunk/4.5?
> 
> 2010-05-31  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/44182
> 	* tree-inline.c (copy_edges_for_bb): Don't split bb if a stmt that
> 	newly needs to end a bb is followed by debug stmts, instead return
> 	true from the function at the end.
> 	(maybe_move_debug_stmts_to_successors): New function.
> 	(copy_cfg_body): Call it if copy_edges_for_bb returned true.
> 	(copy_debug_stmts): Ignore debug stmts with 0 operands.
> 
> 	* g++.dg/debug/pr44182.C: New test.
> 
> --- gcc/tree-inline.c.jj	2010-05-28 00:07:09.000000000 +0200
> +++ gcc/tree-inline.c	2010-05-31 09:01:17.000000000 +0200
> @@ -1836,7 +1836,7 @@ update_ssa_across_abnormal_edges (basic_
>     accordingly.  Edges will be taken care of later.  Assume aux
>     pointers to point to the copies of each BB.  */
>  
> -static void
> +static bool

Please adjust the comment for the changed interface.

>  copy_edges_for_bb (basic_block bb, gcov_type count_scale, basic_block ret_bb)
>  {
>    basic_block new_bb = (basic_block) bb->aux;
> @@ -1844,6 +1844,7 @@ copy_edges_for_bb (basic_block bb, gcov_
>    edge old_edge;
>    gimple_stmt_iterator si;
>    int flags;
> +  bool need_debug_cleanup = false;
>  
>    /* Use the indices from the original blocks to create edges for the
>       new ones.  */
> @@ -1864,7 +1865,7 @@ copy_edges_for_bb (basic_block bb, gcov_
>        }
>  
>    if (bb->index == ENTRY_BLOCK || bb->index == EXIT_BLOCK)
> -    return;
> +    return false;
>  
>    for (si = gsi_start_bb (new_bb); !gsi_end_p (si);)
>      {
> @@ -1899,6 +1900,13 @@ copy_edges_for_bb (basic_block bb, gcov_
>        if (can_throw || nonlocal_goto)
>  	{
>  	  if (!gsi_end_p (si))
> +	    {
> +	      while (!gsi_end_p (si) && is_gimple_debug (gsi_stmt (si)))
> +		gsi_next (&si);
> +	      if (gsi_end_p (si))
> +		need_debug_cleanup = true;
> +	    }
> +	  if (!gsi_end_p (si))
>  	    /* Note that bb's predecessor edges aren't necessarily
>  	       right at this point; split_block doesn't care.  */
>  	    {
> @@ -1923,6 +1931,7 @@ copy_edges_for_bb (basic_block bb, gcov_
>  	update_ssa_across_abnormal_edges (gimple_bb (copy_stmt), ret_bb,
>  					  can_throw, nonlocal_goto);
>      }
> +  return need_debug_cleanup;
>  }
>  
>  /* Copy the PHIs.  All blocks and edges are copied, some blocks
> @@ -2059,6 +2068,58 @@ initialize_cfun (tree new_fndecl, tree c
>    pop_cfun ();
>  }
>  
> +/* Helper function for copy_cfg_body.  Move debug stmts from the end
> +   of NEW_BB to the beginning of successor basic blocks when needed.  If the
> +   successor has multiple predecessors, reset them, otherwise keep
> +   their value.  */
> +
> +static void
> +maybe_move_debug_stmts_to_successors (copy_body_data *id, basic_block new_bb)
> +{
> +  edge e;
> +  edge_iterator ei;
> +  gimple_stmt_iterator si = gsi_last_nondebug_bb (new_bb);
> +
> +  if (gsi_end_p (si)
> +      || gsi_one_before_end_p (si)
> +      || !(stmt_can_throw_internal (gsi_stmt (si))
> +	   || stmt_can_make_abnormal_goto (gsi_stmt (si))))
> +    return;
> +
> +  FOR_EACH_EDGE (e, ei, new_bb->succs)
> +    {
> +      gimple_stmt_iterator ssi = gsi_last_bb (new_bb);
> +      gimple_stmt_iterator dsi = gsi_after_labels (e->dest);
> +      while (is_gimple_debug (gsi_stmt (ssi)))
> +	{
> +	  gimple stmt = gsi_stmt (ssi), new_stmt;
> +	  tree var = gimple_debug_bind_get_var (stmt);
> +	  tree value;
> +
> +	  if (single_pred_p (e->dest))
> +	    {
> +	      value = gimple_debug_bind_get_value (stmt);
> +	      value = unshare_expr (value);
> +	    }
> +	  else
> +	    value = NULL_TREE;
> +	  new_stmt = gimple_build_debug_bind (var, value, stmt);
> +	  gsi_insert_before (&dsi, new_stmt, GSI_SAME_STMT);
> +	  VEC_safe_push (gimple, heap, id->debug_stmts, new_stmt);
> +	  gsi_prev (&ssi);
> +	}
> +    }
> +
> +  gsi_next (&si);
> +  while (!gsi_end_p (si))
> +    {
> +      gimple stmt = gsi_stmt (si);
> +      gsi_remove (&si, true);
> +      /* Tell copy_debug_stmts to ignore it.  */
> +      gimple_set_num_ops (stmt, 0);

I suppose that before fixup-cfg is run there will be no EH edges
for the now throwing stmt, right?

If you are re-setting debug stmts that would be moved into a block
with multiple predecessors then why not, for simplicity, always
reset debug stmts even if you'd now had multiple successors for
new_bb so you can simply move debug stmts instead of re-creating
them and playing games with copy_debug_stmts?

Thanks,
Richard.