> On Jul 6, 2014, at 7:23 AM, Marc Glisse <marc.gli...@inria.fr> wrote: > >> On Mon, 30 Jun 2014, Jeff Law wrote: >> >>> On 06/28/14 16:33, Marc Glisse wrote: >>> In an earlier version of the patch, I was using >>> get_or_create_ssa_default_def (cfun, sym); >>> (I was reusing the same variable). This passed bootstrap+testsuite on >>> all languages except for ada. Indeed, the compiler wanted to coalesce >>> several SSA_NAMEs, including those new ones, in out-of-ssa, but >>> couldn't. >> And that's what you need to delve deeper into. Why did it refuse to >> coalesce? >> >> As long as the lifetimes do not overlap, then coalescing should have worked. > > What is the lifetime of an SSA_NAME with a default definition? The way we > handle it now, we start from the uses and go back to all blocks that can > reach one of the uses, since there is no defining statement where we could > stop (intuitively we should stop where the clobber was, if not earlier). This > huge lifetime makes it very likely for such an SSA_NAME to conflict with > others. And if an abnormal phi is involved, and thus we must coalesce, there > is a problem. > > The patch attached (it should probably use ssa_undefined_value_p with a new > extra argument to say whether we care about partially-undefined) makes the > lifetime of undefined local variables empty and lets the original patch work > with: > def = get_or_create_ssa_default_def (cfun, sym); > instead of creating a new variable. > > However, I am not convinced reusing the same variable is necessarily the best > thing. For warnings, we can create a new variable with the same name (with .1 > added by gcc at the end) and copy the location info (I am not doing that > yet), so little is lost. A new variable expresses more clearly that the value > it holds is random crap. If I reuse the same variable, the SRA patch doesn't > warn because SRA explicitly sets TREE_NO_WARNING (this can probably be > changed, but that's starting to be a lot of changes). Creating a new variable > is also more general. When reading *ssa_name after *ssa_name={v}{CLOBBER}; or > after free(ssa_name); we have no variable to reuse so we will need to create > a new undefined variable, and if a variable is global or a PARM_DECL, its > default definition is not an undefined value (though it will probably happen > in a different pass, so it doesn't have to behave the same). > > (Side note: we don't seem to have any code to notice that: > a=phi<undef,b> > b=phi<undef,a> > means both phis can be replaced with undefined variables.) > > Do you have any advice on the right direction?
The below patch won't work for parameters. Thanks, Andrew > > -- > Marc Glisse > Index: gcc/tree-ssa-live.c > =================================================================== > --- gcc/tree-ssa-live.c (revision 212306) > +++ gcc/tree-ssa-live.c (working copy) > @@ -1086,20 +1086,28 @@ set_var_live_on_entry (tree ssa_name, tr > if (stmt) > { > def_bb = gimple_bb (stmt); > /* Mark defs in liveout bitmap temporarily. */ > if (def_bb) > bitmap_set_bit (&live->liveout[def_bb->index], p); > } > else > def_bb = ENTRY_BLOCK_PTR_FOR_FN (cfun); > > + /* An undefined local variable does not need to be very alive. */ > + if (SSA_NAME_IS_DEFAULT_DEF (ssa_name)) > + { > + tree var = SSA_NAME_VAR (ssa_name); > + if (var && TREE_CODE (var) == VAR_DECL && !is_global_var (var)) > + return; > + } > + > /* Visit each use of SSA_NAME and if it isn't in the same block as the def, > add it to the list of live on entry blocks. */ > FOR_EACH_IMM_USE_FAST (use, imm_iter, ssa_name) > { > gimple use_stmt = USE_STMT (use); > basic_block add_block = NULL; > > if (gimple_code (use_stmt) == GIMPLE_PHI) > { > /* Uses in PHI's are considered to be live at exit of the SRC block > @@ -1422,20 +1430,27 @@ verify_live_on_entry (tree_live_info_p l > fprintf (stderr, "\n"); > } > else > fprintf (stderr, " and there is no default def.\n"); > } > } > } > else > if (d == var) > { > + /* An undefined local variable does not need to be very > + alive. */ > + tree real_var = SSA_NAME_VAR (var); > + if (real_var && TREE_CODE (real_var) == VAR_DECL > + && !is_global_var (real_var)) > + continue; > + > /* The only way this var shouldn't be marked live on entry is > if it occurs in a PHI argument of the block. */ > size_t z; > bool ok = false; > gimple_stmt_iterator gsi; > for (gsi = gsi_start_phis (e->dest); > !gsi_end_p (gsi) && !ok; > gsi_next (&gsi)) > { > gimple phi = gsi_stmt (gsi);