> On Jul 6, 2014, at 7:23 AM, Marc Glisse <[email protected]> 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);