> 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);

Reply via email to