Re: update address taken: don't drop clobbers

[pinskia at gmail dot com]

> On Jul 6, 2014, at 7:23 AM, Marc Glisse <marc.glisse@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);