PR tree-optimzation/31081

PR tree-optimzation/31081

[Jan Hubicka]

Hi,
this testcase in ICE in out-of-ssa pass because we introduce abnormal
PHI with overlapping liferange.  It however just points to the fact that
inlining into loops increase lifetime of uninitialized variables of
function being inlinined.

This patch handle it by initializing to 0 all uninitialized variables
at the beggining of inlined function body.  It is possible to do better
- we can detect strongly connected regions of CFG and do initialization
only in those and we can also do initialization to 0 lazilly, but this
is bit expensive to do (one would need to maintain SCC and dominator
information during inlining that would need some work) and I am not
convinced it is worth the extra compilation time - I didn't measured any
noticeable differences with this patch.

Note that I am not quite convinced that

+ 		  = build_gimple_modify_stmt (new,
+ 				  	      fold_convert (TREE_TYPE (new),
+ 					       		    integer_zero_node));

is safe way to initialize all gimple regs to 0, but I don't know of
better alternative.

Bootstrapped/regtsted i686-linux, OK?

Honza

static int get_record (void);
void f(void);
int g(void);
static int get_record (void)
{
  int     result;
  try
  {
    result = g();
    f();
  }
  catch (const int &)   { }
  return result;
}
int NAV_get_record ( )
{
  int     result;
  for (;;)
    if (get_record ())
      return 1;
}
	PR tree-optimization/31081
	* tree-inline.c (remap_ssa_name): Initialize uninitialized SSA vars to
	0 when inlining and not inlining to first basic block.
	(remap_decl): When var is initialized to 0, don't set default_def.
	(expand_call_inline): Set entry_bb.
	* tree-inline.h (copy_body_data): Add entry_bb.
Index: tree-inline.c
===================================================================
*** tree-inline.c	(revision 131037)
--- tree-inline.c	(working copy)
*************** remap_ssa_name (tree name, copy_body_dat
*** 186,200 ****
      {
        new = make_ssa_name (new, NULL);
        insert_decl_map (id, name, new);
-       if (IS_EMPTY_STMT (SSA_NAME_DEF_STMT (name)))
- 	{
- 	  SSA_NAME_DEF_STMT (new) = build_empty_stmt ();
- 	  if (gimple_default_def (id->src_cfun, SSA_NAME_VAR (name)) == name)
- 	    set_default_def (SSA_NAME_VAR (new), new);
- 	}
        SSA_NAME_OCCURS_IN_ABNORMAL_PHI (new)
  	= SSA_NAME_OCCURS_IN_ABNORMAL_PHI (name);
        TREE_TYPE (new) = TREE_TYPE (SSA_NAME_VAR (new));
      }
    else
      insert_decl_map (id, name, new);
--- 186,227 ----
      {
        new = make_ssa_name (new, NULL);
        insert_decl_map (id, name, new);
        SSA_NAME_OCCURS_IN_ABNORMAL_PHI (new)
  	= SSA_NAME_OCCURS_IN_ABNORMAL_PHI (name);
        TREE_TYPE (new) = TREE_TYPE (SSA_NAME_VAR (new));
+       if (IS_EMPTY_STMT (SSA_NAME_DEF_STMT (name)))
+ 	{
+ 	  /* By inlining function having uninitialized variable, we might
+ 	     extend the lifetime (variable might get reused).  This cause
+ 	     ICE in the case we end up extending lifetime of SSA name across
+ 	     abnormal edge, but also increase register presure.
+ 
+ 	     We simply initialize all uninitialized vars by 0 except for case
+ 	     we are inlining to very first BB.  We can avoid this for all
+ 	     BBs that are not withing strongly connected regions of the CFG,
+ 	     but this is bit expensive to test.
+ 	   */
+ 	  if (id->entry_bb && is_gimple_reg (SSA_NAME_VAR (name))
+ 	      && TREE_CODE (SSA_NAME_VAR (name)) != PARM_DECL
+ 	      && (id->entry_bb != EDGE_SUCC (ENTRY_BLOCK_PTR, 0)
+ 		  || EDGE_COUNT (id->entry_bb->preds) != 1))
+ 	    {
+ 	      block_stmt_iterator bsi = bsi_last (id->entry_bb);
+ 	      tree init_stmt
+ 		  = build_gimple_modify_stmt (new,
+ 				  	      fold_convert (TREE_TYPE (new),
+ 					       		    integer_zero_node));
+ 	      bsi_insert_after (&bsi, init_stmt, BSI_NEW_STMT);
+ 	      SSA_NAME_DEF_STMT (new) = init_stmt;
+ 	      SSA_NAME_IS_DEFAULT_DEF (new) = 0;
+ 	    }
+ 	  else
+ 	    {
+ 	      SSA_NAME_DEF_STMT (new) = build_empty_stmt ();
+ 	      if (gimple_default_def (id->src_cfun, SSA_NAME_VAR (name)) == name)
+ 	        set_default_def (SSA_NAME_VAR (new), new);
+ 	    }
+ 	}
      }
    else
      insert_decl_map (id, name, new);
*************** remap_decl (tree decl, copy_body_data *i
*** 259,265 ****
  	      tree map = remap_ssa_name (def, id);
  	      /* Watch out RESULT_DECLs whose SSA names map directly
  		 to them.  */
! 	      if (TREE_CODE (map) == SSA_NAME)
  	        set_default_def (t, map);
  	    }
  	  add_referenced_var (t);
--- 286,293 ----
  	      tree map = remap_ssa_name (def, id);
  	      /* Watch out RESULT_DECLs whose SSA names map directly
  		 to them.  */
! 	      if (TREE_CODE (map) == SSA_NAME
! 		  && IS_EMPTY_STMT (SSA_NAME_DEF_STMT (map)))
  	        set_default_def (t, map);
  	    }
  	  add_referenced_var (t);
*************** expand_call_inline (basic_block bb, tree
*** 2662,2667 ****
--- 2690,2696 ----
  
    gcc_assert (!id->src_cfun->after_inlining);
  
+   id->entry_bb = bb;
    initialize_inlined_parameters (id, t, fn, bb);
  
    if (DECL_INITIAL (fn))
Index: tree-inline.h
===================================================================
*** tree-inline.h	(revision 131037)
--- tree-inline.h	(working copy)
*************** typedef struct copy_body_data
*** 97,102 ****
--- 97,105 ----
  
    /* Statements that might be possibly folded.  */
    struct pointer_set_t *statements_to_fold;
+ 
+   /* Entry basic block to currently copied body.  */
+   struct basic_block_def *entry_bb;
  } copy_body_data;
  
  /* Weights of constructions for estimate_num_insns.  */

Re: PR tree-optimzation/31081

[Richard Guenther]

On Dec 30, 2007 2:45 PM, Jan Hubicka <jh@suse.cz> wrote:
> Hi,
> this testcase in ICE in out-of-ssa pass because we introduce abnormal
> PHI with overlapping liferange.  It however just points to the fact that
> inlining into loops increase lifetime of uninitialized variables of
> function being inlinined.
>
> This patch handle it by initializing to 0 all uninitialized variables
> at the beggining of inlined function body.  It is possible to do better
> - we can detect strongly connected regions of CFG and do initialization
> only in those and we can also do initialization to 0 lazilly, but this
> is bit expensive to do (one would need to maintain SCC and dominator
> information during inlining that would need some work) and I am not
> convinced it is worth the extra compilation time - I didn't measured any
> noticeable differences with this patch.
>
> Note that I am not quite convinced that
>
> +                 = build_gimple_modify_stmt (new,
> +                                             fold_convert (TREE_TYPE (new),
> +                                                           integer_zero_node));
>
> is safe way to initialize all gimple regs to 0, but I don't know of
> better alternative.

This should work (it's used in other places as well).  I suggested to
create a build_zero_cst () function for this at some point (to get
rid of the special coding in fold_convert), like I introduced
build_one_cst () sometime ago (because that isn't special-cased
in fold_convert ;)).

> Bootstrapped/regtsted i686-linux, OK?

Yes, this look reasonable.  Please watch for fallout.

Thanks,
Richard.

Re: PR tree-optimzation/31081

[Eric Botcazou]

> + 	  if (id->entry_bb && is_gimple_reg (SSA_NAME_VAR (name))
> + 	      && TREE_CODE (SSA_NAME_VAR (name)) != PARM_DECL
> + 	      && (id->entry_bb != EDGE_SUCC (ENTRY_BLOCK_PTR, 0)
> + 		  || EDGE_COUNT (id->entry_bb->preds) != 1))

/home/eric/gnat/gnat-head/src/gcc/tree-inline.c: In function 'remap_ssa_name':
/home/eric/gnat/gnat-head/src/gcc/tree-inline.c:207: error: comparison of 
distinct pointer types lacks a cast
make: *** [tree-inline.o] Error 1

-- 
Eric Botcazou

Re: PR tree-optimzation/31081

[Jan Hubicka]

> > + 	  if (id->entry_bb && is_gimple_reg (SSA_NAME_VAR (name))
> > + 	      && TREE_CODE (SSA_NAME_VAR (name)) != PARM_DECL
> > + 	      && (id->entry_bb != EDGE_SUCC (ENTRY_BLOCK_PTR, 0)
> > + 		  || EDGE_COUNT (id->entry_bb->preds) != 1))
> 
> /home/eric/gnat/gnat-head/src/gcc/tree-inline.c: In function 'remap_ssa_name':
> /home/eric/gnat/gnat-head/src/gcc/tree-inline.c:207: error: comparison of 
> distinct pointer types lacks a cast

Oops, sorry.  I've fixed it now.  I must've swapped final version of
patch with WIP version since I was fixing this bug once already.  I am
re-running testing now, but I hope it was last problem.

Honza
> make: *** [tree-inline.o] Error 1
> 
> -- 
> Eric Botcazou

Re: PR tree-optimzation/31081

[Eric Botcazou]

> Oops, sorry.  I've fixed it now.  I must've swapped final version of
> patch with WIP version since I was fixing this bug once already.  I am
> re-running testing now, but I hope it was last problem.

Thanks.  This seems to work fine now and has fixed the Ada problem as well.

-- 
Eric Botcazou