diff mbox

update address taken: don't drop clobbers

Message ID alpine.DEB.2.11.1410312050430.1802@laptop-mg.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse Nov. 2, 2014, 10:34 a.m. UTC
On Fri, 31 Oct 2014, Richard Biener wrote:

> On Sat, Oct 25, 2014 at 6:29 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> On Fri, 24 Oct 2014, Jeff Law wrote:
>>
>>> I'm starting to agree -- a later message indicated you wanted to drop the
>>> unlink_stmt_vdef call and you wanted to avoid gsi_replace, that seems fine.
>>> I'll approve once those things are taken care of.
>>
>>
>> The following passed bootstrap+testsuite. I wasn't sure what exactly a
>> clobber is guaranteed not to have (no histograms for instance? At least I
>> assumed it doesn't throw) so I may have kept some unnecessary calls when I
>> inlined gsi_replace. I'd be happy to remove any you feel is useless.
>>
>> 2014-10-26  Marc Glisse  <marc.glisse@inria.fr>
>>
>>         PR tree-optimization/60770
>> gcc/
>>         * tree-into-ssa.c: Include value-prof.h.
>>         (maybe_register_def): Replace clobbers with default definitions.
>>         * tree-ssa-live.c: Include tree-ssa.h.
>>         (set_var_live_on_entry): Do not mark undefined variables as live.
>>         (verify_live_on_entry): Do not check undefined variables.
>>         * tree-ssa.h (ssa_undefined_value_p): New parameter for the case
>>         of partially undefined variables.
>>         * tree-ssa.c (ssa_undefined_value_p): Likewise.
>>         (execute_update_addresses_taken): Do not drop clobbers.
>>
>> gcc/testsuite/
>>         * gcc.dg/tree-ssa/pr60770-1.c: New file.
>>
>> --
>> Marc Glisse
>>
>> Index: gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c
>> ===================================================================
>> --- gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c   (revision 0)
>> +++ gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c   (working copy)
>> @@ -0,0 +1,11 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O -Wall" } */
>> +
>> +int f(int n){
>> +  int*p;
>> +  {
>> +    int yyy=n;
>> +    p=&yyy;
>> +  }
>> +  return *p; /* { dg-warning "yyy" } */
>> +}
>> Index: gcc/tree-into-ssa.c
>> ===================================================================
>> --- gcc/tree-into-ssa.c (revision 216689)
>> +++ gcc/tree-into-ssa.c (working copy)
>> @@ -52,20 +52,21 @@ along with GCC; see the file COPYING3.
>>  #include "expr.h"
>>  #include "tree-dfa.h"
>>  #include "tree-ssa.h"
>>  #include "tree-inline.h"
>>  #include "tree-pass.h"
>>  #include "cfgloop.h"
>>  #include "domwalk.h"
>>  #include "params.h"
>>  #include "diagnostic-core.h"
>>  #include "tree-into-ssa.h"
>> +#include "value-prof.h"
>>
>>  #define PERCENT(x,y) ((float)(x) * 100.0 / (float)(y))
>>
>>  /* This file builds the SSA form for a function as described in:
>>     R. Cytron, J. Ferrante, B. Rosen, M. Wegman, and K. Zadeck. Efficiently
>>     Computing Static Single Assignment Form and the Control Dependence
>>     Graph. ACM Transactions on Programming Languages and Systems,
>>     13(4):451-490, October 1991.  */
>>
>>  /* Structure to map a variable VAR to the set of blocks that contain
>> @@ -1837,26 +1838,42 @@ maybe_register_def (def_operand_p def_p,
>>  {
>>    tree def = DEF_FROM_PTR (def_p);
>>    tree sym = DECL_P (def) ? def : SSA_NAME_VAR (def);
>>
>>    /* If DEF is a naked symbol that needs renaming, create a new
>>       name for it.  */
>>    if (marked_for_renaming (sym))
>>      {
>>        if (DECL_P (def))
>>         {
>> -         tree tracked_var;
>> +         if (gimple_clobber_p (stmt) && is_gimple_reg (sym))
>
> I think you know that sym is a gimple-reg as the code previously
> unconditionally generated an SSA name for it.

I checked that in July and it failed:
https://gcc.gnu.org/ml/gcc-patches/2014-07/msg01828.html

>> +           {
>> +             /* Replace clobber stmts with a default def. This new use of a
>> +                default definition may make it look like SSA_NAMEs have
>> +                conflicting lifetimes, so we need special code to let them
>> +                coalesce properly.  */
>> +             /* Hand-inlined version of the following, for safety
>> +                gsi_replace (&gsi, gimple_build_nop (), true);  */
>> +             gimple nop = gimple_build_nop ();
>> +             gimple_set_bb (nop, gsi_bb (gsi));
>> +             gimple_set_bb (stmt, NULL);
>> +             gimple_remove_stmt_histograms (cfun, stmt);
>> +             delink_stmt_imm_use (stmt);
>> +             gsi_set_stmt (&gsi, nop);
>
> Is there any reason for this dance?  I'd rather have maybe_register_def
> return a bool whether to remove the stmt... passing it down to the
> single caller of rewrite_update_stmt which can then gsi_remove the
> stmt.

For more context, my starting point was the code in rewrite_stmt, which
I was trying to port to rewrite_update_stmt (and thus
maybe_register_def):

         if (gimple_clobber_p (stmt)
             && is_gimple_reg (var))
           {
             /* If we rewrite a DECL into SSA form then drop its
                clobber stmts and replace uses with a new default def.  */
             gcc_checking_assert (TREE_CODE (var) == VAR_DECL
                                  && !gimple_vdef (stmt));
             gsi_replace (si, gimple_build_nop (), true);
             register_new_def (get_or_create_ssa_default_def (cfun, var), var);
             break;
           }

I would be happy using the same gsi_replace line, but you said that it
is dangerous because it runs update_stmt (though I don't see what bad
thing may happen when replacing a clobber by a nop), so I tried to do
the same thing as gsi_replace without update_stmt... Though now that I 
re-read https://gcc.gnu.org/ml/gcc-patches/2014-07/msg01656.html it is not 
clear that you are really opposed to the gsi_replace version.

I tried again with gsi_remove, trying to understand why it was failing 
before, and that is because I was calling it in maybe_register_def instead 
of delegating to whoever calls gsi_next.

>> -         def = make_ssa_name (def, stmt);
>> +             def = get_or_create_ssa_default_def (cfun, sym);
>
> I think if 'def' turns out to be a PARM_DECL this does the wrong
> thing (well, not technically wrong...  but maybe unexpected).  Not
> sure if we ever end up with a PARM = {} clobber though.  Maybe
> guard all this with TREE_CODE (def) == VAR_DECL for extra
> safety.

Hmm, you are right. The rewrite_stmt version has

             gcc_checking_assert (TREE_CODE (var) == VAR_DECL && ...

I don't remember exactly why I removed it, maybe because the second part 
of the assertion was failing.

Here is a new version, that passed bootstrap+testsuite on x86_64-linux-gnu.

2014-11-03  Marc Glisse  <marc.glisse@inria.fr>

 	PR tree-optimization/60770
gcc/
 	* tree-into-ssa.c (rewrite_update_stmt): Return whether the
 	statement should be removed.
 	(maybe_register_def): Likewise. Replace clobbers with default
 	definitions.
 	(rewrite_dom_walker::before_dom_children): Remove statement if
 	rewrite_update_stmt says so.
 	* tree-ssa-live.c: Include tree-ssa.h.
 	(set_var_live_on_entry): Do not mark undefined variables as live.
 	(verify_live_on_entry): Do not check undefined variables.
 	* tree-ssa.h (ssa_undefined_value_p): New parameter for the case
 	of partially undefined variables.
 	* tree-ssa.c (ssa_undefined_value_p): Likewise.
 	(execute_update_addresses_taken): Do not drop clobbers.

gcc/testsuite/
 	* gcc.dg/tree-ssa/pr60770-1.c: New file.

Comments

Richard Biener Nov. 3, 2014, 9:06 a.m. UTC | #1
On Sun, Nov 2, 2014 at 11:34 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Fri, 31 Oct 2014, Richard Biener wrote:
>
>> On Sat, Oct 25, 2014 at 6:29 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>
>>> On Fri, 24 Oct 2014, Jeff Law wrote:
>>>
>>>> I'm starting to agree -- a later message indicated you wanted to drop
>>>> the
>>>> unlink_stmt_vdef call and you wanted to avoid gsi_replace, that seems
>>>> fine.
>>>> I'll approve once those things are taken care of.
>>>
>>>
>>>
>>> The following passed bootstrap+testsuite. I wasn't sure what exactly a
>>> clobber is guaranteed not to have (no histograms for instance? At least I
>>> assumed it doesn't throw) so I may have kept some unnecessary calls when
>>> I
>>> inlined gsi_replace. I'd be happy to remove any you feel is useless.
>>>
>>> 2014-10-26  Marc Glisse  <marc.glisse@inria.fr>
>>>
>>>         PR tree-optimization/60770
>>> gcc/
>>>         * tree-into-ssa.c: Include value-prof.h.
>>>         (maybe_register_def): Replace clobbers with default definitions.
>>>         * tree-ssa-live.c: Include tree-ssa.h.
>>>         (set_var_live_on_entry): Do not mark undefined variables as live.
>>>         (verify_live_on_entry): Do not check undefined variables.
>>>         * tree-ssa.h (ssa_undefined_value_p): New parameter for the case
>>>         of partially undefined variables.
>>>         * tree-ssa.c (ssa_undefined_value_p): Likewise.
>>>         (execute_update_addresses_taken): Do not drop clobbers.
>>>
>>> gcc/testsuite/
>>>         * gcc.dg/tree-ssa/pr60770-1.c: New file.
>>>
>>> --
>>> Marc Glisse
>>>
>>> Index: gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c
>>> ===================================================================
>>> --- gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c   (revision 0)
>>> +++ gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c   (working copy)
>>> @@ -0,0 +1,11 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O -Wall" } */
>>> +
>>> +int f(int n){
>>> +  int*p;
>>> +  {
>>> +    int yyy=n;
>>> +    p=&yyy;
>>> +  }
>>> +  return *p; /* { dg-warning "yyy" } */
>>> +}
>>> Index: gcc/tree-into-ssa.c
>>> ===================================================================
>>> --- gcc/tree-into-ssa.c (revision 216689)
>>> +++ gcc/tree-into-ssa.c (working copy)
>>> @@ -52,20 +52,21 @@ along with GCC; see the file COPYING3.
>>>  #include "expr.h"
>>>  #include "tree-dfa.h"
>>>  #include "tree-ssa.h"
>>>  #include "tree-inline.h"
>>>  #include "tree-pass.h"
>>>  #include "cfgloop.h"
>>>  #include "domwalk.h"
>>>  #include "params.h"
>>>  #include "diagnostic-core.h"
>>>  #include "tree-into-ssa.h"
>>> +#include "value-prof.h"
>>>
>>>  #define PERCENT(x,y) ((float)(x) * 100.0 / (float)(y))
>>>
>>>  /* This file builds the SSA form for a function as described in:
>>>     R. Cytron, J. Ferrante, B. Rosen, M. Wegman, and K. Zadeck.
>>> Efficiently
>>>     Computing Static Single Assignment Form and the Control Dependence
>>>     Graph. ACM Transactions on Programming Languages and Systems,
>>>     13(4):451-490, October 1991.  */
>>>
>>>  /* Structure to map a variable VAR to the set of blocks that contain
>>> @@ -1837,26 +1838,42 @@ maybe_register_def (def_operand_p def_p,
>>>  {
>>>    tree def = DEF_FROM_PTR (def_p);
>>>    tree sym = DECL_P (def) ? def : SSA_NAME_VAR (def);
>>>
>>>    /* If DEF is a naked symbol that needs renaming, create a new
>>>       name for it.  */
>>>    if (marked_for_renaming (sym))
>>>      {
>>>        if (DECL_P (def))
>>>         {
>>> -         tree tracked_var;
>>> +         if (gimple_clobber_p (stmt) && is_gimple_reg (sym))
>>
>>
>> I think you know that sym is a gimple-reg as the code previously
>> unconditionally generated an SSA name for it.
>
>
> I checked that in July and it failed:
> https://gcc.gnu.org/ml/gcc-patches/2014-07/msg01828.html
>
>>> +           {
>>> +             /* Replace clobber stmts with a default def. This new use
>>> of a
>>> +                default definition may make it look like SSA_NAMEs have
>>> +                conflicting lifetimes, so we need special code to let
>>> them
>>> +                coalesce properly.  */
>>> +             /* Hand-inlined version of the following, for safety
>>> +                gsi_replace (&gsi, gimple_build_nop (), true);  */
>>> +             gimple nop = gimple_build_nop ();
>>> +             gimple_set_bb (nop, gsi_bb (gsi));
>>> +             gimple_set_bb (stmt, NULL);
>>> +             gimple_remove_stmt_histograms (cfun, stmt);
>>> +             delink_stmt_imm_use (stmt);
>>> +             gsi_set_stmt (&gsi, nop);
>>
>>
>> Is there any reason for this dance?  I'd rather have maybe_register_def
>> return a bool whether to remove the stmt... passing it down to the
>> single caller of rewrite_update_stmt which can then gsi_remove the
>> stmt.
>
>
> For more context, my starting point was the code in rewrite_stmt, which
> I was trying to port to rewrite_update_stmt (and thus
> maybe_register_def):
>
>         if (gimple_clobber_p (stmt)
>             && is_gimple_reg (var))
>           {
>             /* If we rewrite a DECL into SSA form then drop its
>                clobber stmts and replace uses with a new default def.  */
>             gcc_checking_assert (TREE_CODE (var) == VAR_DECL
>                                  && !gimple_vdef (stmt));
>             gsi_replace (si, gimple_build_nop (), true);
>             register_new_def (get_or_create_ssa_default_def (cfun, var),
> var);
>             break;
>           }
>
> I would be happy using the same gsi_replace line, but you said that it
> is dangerous because it runs update_stmt (though I don't see what bad
> thing may happen when replacing a clobber by a nop), so I tried to do
> the same thing as gsi_replace without update_stmt... Though now that I
> re-read https://gcc.gnu.org/ml/gcc-patches/2014-07/msg01656.html it is not
> clear that you are really opposed to the gsi_replace version.
>
> I tried again with gsi_remove, trying to understand why it was failing
> before, and that is because I was calling it in maybe_register_def instead
> of delegating to whoever calls gsi_next.
>
>>> -         def = make_ssa_name (def, stmt);
>>> +             def = get_or_create_ssa_default_def (cfun, sym);
>>
>>
>> I think if 'def' turns out to be a PARM_DECL this does the wrong
>> thing (well, not technically wrong...  but maybe unexpected).  Not
>> sure if we ever end up with a PARM = {} clobber though.  Maybe
>> guard all this with TREE_CODE (def) == VAR_DECL for extra
>> safety.
>
>
> Hmm, you are right. The rewrite_stmt version has
>
>             gcc_checking_assert (TREE_CODE (var) == VAR_DECL && ...
>
> I don't remember exactly why I removed it, maybe because the second part of
> the assertion was failing.
>
> Here is a new version, that passed bootstrap+testsuite on x86_64-linux-gnu.

Ok.

Thanks,
Richard.

> 2014-11-03  Marc Glisse  <marc.glisse@inria.fr>
>
>         PR tree-optimization/60770
> gcc/
>         * tree-into-ssa.c (rewrite_update_stmt): Return whether the
>         statement should be removed.
>         (maybe_register_def): Likewise. Replace clobbers with default
>         definitions.
>         (rewrite_dom_walker::before_dom_children): Remove statement if
>         rewrite_update_stmt says so.
>
>         * tree-ssa-live.c: Include tree-ssa.h.
>         (set_var_live_on_entry): Do not mark undefined variables as live.
>         (verify_live_on_entry): Do not check undefined variables.
>         * tree-ssa.h (ssa_undefined_value_p): New parameter for the case
>         of partially undefined variables.
>         * tree-ssa.c (ssa_undefined_value_p): Likewise.
>         (execute_update_addresses_taken): Do not drop clobbers.
>
> gcc/testsuite/
>         * gcc.dg/tree-ssa/pr60770-1.c: New file.
>
> --
> Marc Glisse
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c   (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c   (working copy)
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -Wall" } */
> +
> +int f(int n){
> +  int*p;
> +  {
> +    int yyy=n;
> +    p=&yyy;
> +  }
> +  return *p; /* { dg-warning "yyy" } */
> +}
> Index: gcc/tree-into-ssa.c
> ===================================================================
> --- gcc/tree-into-ssa.c (revision 217007)
> +++ gcc/tree-into-ssa.c (working copy)
> @@ -1826,41 +1826,51 @@ maybe_replace_use_in_debug_stmt (use_ope
>    if (rdef && rdef != use)
>      SET_USE (use_p, rdef);
>
>    return rdef != NULL_TREE;
>  }
>
>
>  /* If the operand pointed to by DEF_P is an SSA name in NEW_SSA_NAMES
>     or OLD_SSA_NAMES, or if it is a symbol marked for renaming,
>     register it as the current definition for the names replaced by
> -   DEF_P.  */
> +   DEF_P.  Returns whether the statement should be removed.  */
>
> -static inline void
> +static inline bool
>  maybe_register_def (def_operand_p def_p, gimple stmt,
>                     gimple_stmt_iterator gsi)
>  {
>    tree def = DEF_FROM_PTR (def_p);
>    tree sym = DECL_P (def) ? def : SSA_NAME_VAR (def);
> +  bool to_delete = false;
>
>    /* If DEF is a naked symbol that needs renaming, create a new
>       name for it.  */
>    if (marked_for_renaming (sym))
>      {
>        if (DECL_P (def))
>         {
> -         tree tracked_var;
> -
> -         def = make_ssa_name (def, stmt);
> +         if (gimple_clobber_p (stmt) && is_gimple_reg (sym))
> +           {
> +             gcc_checking_assert (TREE_CODE (sym) == VAR_DECL);
> +             /* Replace clobber stmts with a default def. This new use of a
> +                default definition may make it look like SSA_NAMEs have
> +                conflicting lifetimes, so we need special code to let them
> +                coalesce properly.  */
> +             to_delete = true;
> +             def = get_or_create_ssa_default_def (cfun, sym);
> +           }
> +         else
> +           def = make_ssa_name (def, stmt);
>           SET_DEF (def_p, def);
>
> -         tracked_var = target_for_debug_bind (sym);
> +         tree tracked_var = target_for_debug_bind (sym);
>           if (tracked_var)
>             {
>               gimple note = gimple_build_debug_bind (tracked_var, def,
> stmt);
>               /* If stmt ends the bb, insert the debug stmt on the single
>                  non-EH edge from the stmt.  */
>               if (gsi_one_before_end_p (gsi) && stmt_ends_bb_p (stmt))
>                 {
>                   basic_block bb = gsi_bb (gsi);
>                   edge_iterator ei;
>                   edge e, ef = NULL;
> @@ -1904,40 +1914,42 @@ maybe_register_def (def_operand_p def_p,
>        /* If DEF is a new name, register it as a new definition
>          for all the names replaced by DEF.  */
>        if (is_new_name (def))
>         register_new_update_set (def, names_replaced_by (def));
>
>        /* If DEF is an old name, register DEF as a new
>          definition for itself.  */
>        if (is_old_name (def))
>         register_new_update_single (def, def);
>      }
> +
> +  return to_delete;
>  }
>
>
>  /* Update every variable used in the statement pointed-to by SI.  The
>     statement is assumed to be in SSA form already.  Names in
>     OLD_SSA_NAMES used by SI will be updated to their current reaching
>     definition.  Names in OLD_SSA_NAMES or NEW_SSA_NAMES defined by SI
>     will be registered as a new definition for their corresponding name
> -   in OLD_SSA_NAMES.  */
> +   in OLD_SSA_NAMES.  Returns whether STMT should be removed.  */
>
> -static void
> +static bool
>  rewrite_update_stmt (gimple stmt, gimple_stmt_iterator gsi)
>  {
>    use_operand_p use_p;
>    def_operand_p def_p;
>    ssa_op_iter iter;
>
>    /* Only update marked statements.  */
>    if (!rewrite_uses_p (stmt) && !register_defs_p (stmt))
> -    return;
> +    return false;
>
>    if (dump_file && (dump_flags & TDF_DETAILS))
>      {
>        fprintf (dump_file, "Updating SSA information for statement ");
>        print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
>      }
>
>    /* Rewrite USES included in OLD_SSA_NAMES and USES whose underlying
>       symbol is marked for renaming.  */
>    if (rewrite_uses_p (stmt))
> @@ -1974,23 +1986,26 @@ rewrite_update_stmt (gimple stmt, gimple
>        else
>         {
>           FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_ALL_USES)
>             maybe_replace_use (use_p);
>         }
>      }
>
>    /* Register definitions of names in NEW_SSA_NAMES and OLD_SSA_NAMES.
>       Also register definitions for names whose underlying symbol is
>       marked for renaming.  */
> +  bool to_delete = false;
>    if (register_defs_p (stmt))
>      FOR_EACH_SSA_DEF_OPERAND (def_p, stmt, iter, SSA_OP_ALL_DEFS)
> -      maybe_register_def (def_p, stmt, gsi);
> +      to_delete |= maybe_register_def (def_p, stmt, gsi);
> +
> +  return to_delete;
>  }
>
>
>  /* Visit all the successor blocks of BB looking for PHI nodes.  For
>     every PHI node found, check if any of its arguments is in
>     OLD_SSA_NAMES.  If so, and if the argument has a current reaching
>     definition, replace it.  */
>
>  static void
>  rewrite_update_phi_arguments (basic_block bb)
> @@ -2142,22 +2157,25 @@ rewrite_update_dom_walker::before_dom_ch
>         }
>
>        if (is_abnormal_phi)
>         SSA_NAME_OCCURS_IN_ABNORMAL_PHI (lhs) = 1;
>      }
>
>    /* Step 2.  Rewrite every variable used in each statement in the block.
> */
>    if (bitmap_bit_p (interesting_blocks, bb->index))
>      {
>        gcc_checking_assert (bitmap_bit_p (blocks_to_update, bb->index));
> -      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> -        rewrite_update_stmt (gsi_stmt (gsi), gsi);
> +      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
> +       if (rewrite_update_stmt (gsi_stmt (gsi), gsi))
> +         gsi_remove (&gsi, true);
> +       else
> +         gsi_next (&gsi);
>      }
>
>    /* Step 3.  Update PHI nodes.  */
>    rewrite_update_phi_arguments (bb);
>  }
>
>  /* Called after visiting block BB.  Unwind BLOCK_DEFS_STACK to restore
>     the current reaching definition of every name re-written in BB to
>     the original reaching definition before visiting BB.  This
>     unwinding must be done in the opposite order to what is done in
> Index: gcc/tree-ssa-live.c
> ===================================================================
> --- gcc/tree-ssa-live.c (revision 217007)
> +++ gcc/tree-ssa-live.c (working copy)
> @@ -50,20 +50,21 @@ along with GCC; see the file COPYING3.
>  #include "stringpool.h"
>  #include "tree-ssanames.h"
>  #include "expr.h"
>  #include "tree-dfa.h"
>  #include "timevar.h"
>  #include "dumpfile.h"
>  #include "tree-ssa-live.h"
>  #include "diagnostic-core.h"
>  #include "debug.h"
>  #include "flags.h"
> +#include "tree-ssa.h"
>
>  #ifdef ENABLE_CHECKING
>  static void  verify_live_on_entry (tree_live_info_p);
>  #endif
>
>
>  /* VARMAP maintains a mapping from SSA version number to real variables.
>
>     All SSA_NAMES are divided into partitions.  Initially each ssa_name is
> the
>     only member of it's own partition.  Coalescing will attempt to group any
> @@ -1096,20 +1097,24 @@ 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_undefined_value_p (ssa_name, false))
> +    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
> @@ -1432,20 +1437,25 @@ 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.  */
> +               if (ssa_undefined_value_p (var, false))
> +                 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);
> Index: gcc/tree-ssa.c
> ===================================================================
> --- gcc/tree-ssa.c      (revision 217007)
> +++ gcc/tree-ssa.c      (working copy)
> @@ -1181,24 +1181,25 @@ tree_ssa_useless_type_conversion (tree e
>
>  tree
>  tree_ssa_strip_useless_type_conversions (tree exp)
>  {
>    while (tree_ssa_useless_type_conversion (exp))
>      exp = TREE_OPERAND (exp, 0);
>    return exp;
>  }
>
>
> -/* Return true if T, an SSA_NAME, has an undefined value.  */
> +/* Return true if T, an SSA_NAME, has an undefined value.  PARTIAL is what
> +   should be returned if the value is only partially undefined.  */
>
>  bool
> -ssa_undefined_value_p (tree t)
> +ssa_undefined_value_p (tree t, bool partial)
>  {
>    gimple def_stmt;
>    tree var = SSA_NAME_VAR (t);
>
>    if (!var)
>      ;
>    /* Parameters get their initial value from the function entry.  */
>    else if (TREE_CODE (var) == PARM_DECL)
>      return false;
>    /* When returning by reference the return address is actually a hidden
> @@ -1208,21 +1209,21 @@ ssa_undefined_value_p (tree t)
>    /* Hard register variables get their initial value from the ether.  */
>    else if (TREE_CODE (var) == VAR_DECL && DECL_HARD_REGISTER (var))
>      return false;
>
>    /* The value is undefined iff its definition statement is empty.  */
>    def_stmt = SSA_NAME_DEF_STMT (t);
>    if (gimple_nop_p (def_stmt))
>      return true;
>
>    /* Check if the complex was not only partially defined.  */
> -  if (is_gimple_assign (def_stmt)
> +  if (partial && is_gimple_assign (def_stmt)
>        && gimple_assign_rhs_code (def_stmt) == COMPLEX_EXPR)
>      {
>        tree rhs1, rhs2;
>
>        rhs1 = gimple_assign_rhs1 (def_stmt);
>        rhs2 = gimple_assign_rhs2 (def_stmt);
>        return (TREE_CODE (rhs1) == SSA_NAME && ssa_undefined_value_p (rhs1))
>              || (TREE_CODE (rhs2) == SSA_NAME && ssa_undefined_value_p
> (rhs2));
>      }
>    return false;
> @@ -1554,32 +1555,20 @@ execute_update_addresses_taken (void)
>                 rhs = gimple_assign_rhs1 (stmt);
>                 if (gimple_assign_lhs (stmt) != lhs
>                     && !useless_type_conversion_p (TREE_TYPE (lhs),
>                                                    TREE_TYPE (rhs)))
>                   rhs = fold_build1 (VIEW_CONVERT_EXPR,
>                                      TREE_TYPE (lhs), rhs);
>
>                 if (gimple_assign_lhs (stmt) != lhs)
>                   gimple_assign_set_lhs (stmt, lhs);
>
> -               /* For var ={v} {CLOBBER}; where var lost
> -                  TREE_ADDRESSABLE just remove the stmt.  */
> -               if (DECL_P (lhs)
> -                   && TREE_CLOBBER_P (rhs)
> -                   && bitmap_bit_p (suitable_for_renaming, DECL_UID (lhs)))
> -                 {
> -                   unlink_stmt_vdef (stmt);
> -                   gsi_remove (&gsi, true);
> -                   release_defs (stmt);
> -                   continue;
> -                 }
> -
>                 if (gimple_assign_rhs1 (stmt) != rhs)
>                   {
>                     gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
>                     gimple_assign_set_rhs_from_tree (&gsi, rhs);
>                   }
>               }
>
>             else if (gimple_code (stmt) == GIMPLE_CALL)
>               {
>                 unsigned i;
> Index: gcc/tree-ssa.h
> ===================================================================
> --- gcc/tree-ssa.h      (revision 217007)
> +++ gcc/tree-ssa.h      (working copy)
> @@ -44,21 +44,21 @@ extern tree target_for_debug_bind (tree)
>  extern void insert_debug_temp_for_var_def (gimple_stmt_iterator *, tree);
>  extern void insert_debug_temps_for_defs (gimple_stmt_iterator *);
>  extern void reset_debug_uses (gimple);
>  extern void release_defs_bitset (bitmap toremove);
>  extern void verify_ssa (bool, bool);
>  extern void init_tree_ssa (struct function *);
>  extern void delete_tree_ssa (void);
>  extern bool tree_ssa_useless_type_conversion (tree);
>  extern tree tree_ssa_strip_useless_type_conversions (tree);
>
> -extern bool ssa_undefined_value_p (tree);
> +extern bool ssa_undefined_value_p (tree, bool = true);
>  extern void execute_update_addresses_taken (void);
>
>  /* Given an edge_var_map V, return the PHI arg definition.  */
>
>  static inline tree
>  redirect_edge_var_map_def (edge_var_map *v)
>  {
>    return v->def;
>  }
>
>
diff mbox

Patch

Index: gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr60770-1.c	(working copy)
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -Wall" } */
+
+int f(int n){
+  int*p;
+  {
+    int yyy=n;
+    p=&yyy;
+  }
+  return *p; /* { dg-warning "yyy" } */
+}
Index: gcc/tree-into-ssa.c
===================================================================
--- gcc/tree-into-ssa.c	(revision 217007)
+++ gcc/tree-into-ssa.c	(working copy)
@@ -1826,41 +1826,51 @@  maybe_replace_use_in_debug_stmt (use_ope
   if (rdef && rdef != use)
     SET_USE (use_p, rdef);
 
   return rdef != NULL_TREE;
 }
 
 
 /* If the operand pointed to by DEF_P is an SSA name in NEW_SSA_NAMES
    or OLD_SSA_NAMES, or if it is a symbol marked for renaming,
    register it as the current definition for the names replaced by
-   DEF_P.  */
+   DEF_P.  Returns whether the statement should be removed.  */
 
-static inline void
+static inline bool
 maybe_register_def (def_operand_p def_p, gimple stmt,
 		    gimple_stmt_iterator gsi)
 {
   tree def = DEF_FROM_PTR (def_p);
   tree sym = DECL_P (def) ? def : SSA_NAME_VAR (def);
+  bool to_delete = false;
 
   /* If DEF is a naked symbol that needs renaming, create a new
      name for it.  */
   if (marked_for_renaming (sym))
     {
       if (DECL_P (def))
 	{
-	  tree tracked_var;
-
-	  def = make_ssa_name (def, stmt);
+	  if (gimple_clobber_p (stmt) && is_gimple_reg (sym))
+	    {
+	      gcc_checking_assert (TREE_CODE (sym) == VAR_DECL);
+	      /* Replace clobber stmts with a default def. This new use of a
+		 default definition may make it look like SSA_NAMEs have
+		 conflicting lifetimes, so we need special code to let them
+		 coalesce properly.  */
+	      to_delete = true;
+	      def = get_or_create_ssa_default_def (cfun, sym);
+	    }
+	  else
+	    def = make_ssa_name (def, stmt);
 	  SET_DEF (def_p, def);
 
-	  tracked_var = target_for_debug_bind (sym);
+	  tree tracked_var = target_for_debug_bind (sym);
 	  if (tracked_var)
 	    {
 	      gimple note = gimple_build_debug_bind (tracked_var, def, stmt);
 	      /* If stmt ends the bb, insert the debug stmt on the single
 		 non-EH edge from the stmt.  */
 	      if (gsi_one_before_end_p (gsi) && stmt_ends_bb_p (stmt))
 		{
 		  basic_block bb = gsi_bb (gsi);
 		  edge_iterator ei;
 		  edge e, ef = NULL;
@@ -1904,40 +1914,42 @@  maybe_register_def (def_operand_p def_p,
       /* If DEF is a new name, register it as a new definition
 	 for all the names replaced by DEF.  */
       if (is_new_name (def))
 	register_new_update_set (def, names_replaced_by (def));
 
       /* If DEF is an old name, register DEF as a new
 	 definition for itself.  */
       if (is_old_name (def))
 	register_new_update_single (def, def);
     }
+
+  return to_delete;
 }
 
 
 /* Update every variable used in the statement pointed-to by SI.  The
    statement is assumed to be in SSA form already.  Names in
    OLD_SSA_NAMES used by SI will be updated to their current reaching
    definition.  Names in OLD_SSA_NAMES or NEW_SSA_NAMES defined by SI
    will be registered as a new definition for their corresponding name
-   in OLD_SSA_NAMES.  */
+   in OLD_SSA_NAMES.  Returns whether STMT should be removed.  */
 
-static void
+static bool
 rewrite_update_stmt (gimple stmt, gimple_stmt_iterator gsi)
 {
   use_operand_p use_p;
   def_operand_p def_p;
   ssa_op_iter iter;
 
   /* Only update marked statements.  */
   if (!rewrite_uses_p (stmt) && !register_defs_p (stmt))
-    return;
+    return false;
 
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "Updating SSA information for statement ");
       print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
     }
 
   /* Rewrite USES included in OLD_SSA_NAMES and USES whose underlying
      symbol is marked for renaming.  */
   if (rewrite_uses_p (stmt))
@@ -1974,23 +1986,26 @@  rewrite_update_stmt (gimple stmt, gimple
       else
 	{
 	  FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_ALL_USES)
 	    maybe_replace_use (use_p);
 	}
     }
 
   /* Register definitions of names in NEW_SSA_NAMES and OLD_SSA_NAMES.
      Also register definitions for names whose underlying symbol is
      marked for renaming.  */
+  bool to_delete = false;
   if (register_defs_p (stmt))
     FOR_EACH_SSA_DEF_OPERAND (def_p, stmt, iter, SSA_OP_ALL_DEFS)
-      maybe_register_def (def_p, stmt, gsi);
+      to_delete |= maybe_register_def (def_p, stmt, gsi);
+
+  return to_delete;
 }
 
 
 /* Visit all the successor blocks of BB looking for PHI nodes.  For
    every PHI node found, check if any of its arguments is in
    OLD_SSA_NAMES.  If so, and if the argument has a current reaching
    definition, replace it.  */
 
 static void
 rewrite_update_phi_arguments (basic_block bb)
@@ -2142,22 +2157,25 @@  rewrite_update_dom_walker::before_dom_ch
 	}
 
       if (is_abnormal_phi)
 	SSA_NAME_OCCURS_IN_ABNORMAL_PHI (lhs) = 1;
     }
 
   /* Step 2.  Rewrite every variable used in each statement in the block.  */
   if (bitmap_bit_p (interesting_blocks, bb->index))
     {
       gcc_checking_assert (bitmap_bit_p (blocks_to_update, bb->index));
-      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-        rewrite_update_stmt (gsi_stmt (gsi), gsi);
+      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
+	if (rewrite_update_stmt (gsi_stmt (gsi), gsi))
+	  gsi_remove (&gsi, true);
+	else
+	  gsi_next (&gsi);
     }
 
   /* Step 3.  Update PHI nodes.  */
   rewrite_update_phi_arguments (bb);
 }
 
 /* Called after visiting block BB.  Unwind BLOCK_DEFS_STACK to restore
    the current reaching definition of every name re-written in BB to
    the original reaching definition before visiting BB.  This
    unwinding must be done in the opposite order to what is done in
Index: gcc/tree-ssa-live.c
===================================================================
--- gcc/tree-ssa-live.c	(revision 217007)
+++ gcc/tree-ssa-live.c	(working copy)
@@ -50,20 +50,21 @@  along with GCC; see the file COPYING3.
 #include "stringpool.h"
 #include "tree-ssanames.h"
 #include "expr.h"
 #include "tree-dfa.h"
 #include "timevar.h"
 #include "dumpfile.h"
 #include "tree-ssa-live.h"
 #include "diagnostic-core.h"
 #include "debug.h"
 #include "flags.h"
+#include "tree-ssa.h"
 
 #ifdef ENABLE_CHECKING
 static void  verify_live_on_entry (tree_live_info_p);
 #endif
 
 
 /* VARMAP maintains a mapping from SSA version number to real variables.
 
    All SSA_NAMES are divided into partitions.  Initially each ssa_name is the
    only member of it's own partition.  Coalescing will attempt to group any
@@ -1096,20 +1097,24 @@  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_undefined_value_p (ssa_name, false))
+    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
@@ -1432,20 +1437,25 @@  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.  */
+		if (ssa_undefined_value_p (var, false))
+		  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);
Index: gcc/tree-ssa.c
===================================================================
--- gcc/tree-ssa.c	(revision 217007)
+++ gcc/tree-ssa.c	(working copy)
@@ -1181,24 +1181,25 @@  tree_ssa_useless_type_conversion (tree e
 
 tree
 tree_ssa_strip_useless_type_conversions (tree exp)
 {
   while (tree_ssa_useless_type_conversion (exp))
     exp = TREE_OPERAND (exp, 0);
   return exp;
 }
 
 
-/* Return true if T, an SSA_NAME, has an undefined value.  */
+/* Return true if T, an SSA_NAME, has an undefined value.  PARTIAL is what
+   should be returned if the value is only partially undefined.  */
 
 bool
-ssa_undefined_value_p (tree t)
+ssa_undefined_value_p (tree t, bool partial)
 {
   gimple def_stmt;
   tree var = SSA_NAME_VAR (t);
 
   if (!var)
     ;
   /* Parameters get their initial value from the function entry.  */
   else if (TREE_CODE (var) == PARM_DECL)
     return false;
   /* When returning by reference the return address is actually a hidden
@@ -1208,21 +1209,21 @@  ssa_undefined_value_p (tree t)
   /* Hard register variables get their initial value from the ether.  */
   else if (TREE_CODE (var) == VAR_DECL && DECL_HARD_REGISTER (var))
     return false;
 
   /* The value is undefined iff its definition statement is empty.  */
   def_stmt = SSA_NAME_DEF_STMT (t);
   if (gimple_nop_p (def_stmt))
     return true;
 
   /* Check if the complex was not only partially defined.  */
-  if (is_gimple_assign (def_stmt)
+  if (partial && is_gimple_assign (def_stmt)
       && gimple_assign_rhs_code (def_stmt) == COMPLEX_EXPR)
     {
       tree rhs1, rhs2;
 
       rhs1 = gimple_assign_rhs1 (def_stmt);
       rhs2 = gimple_assign_rhs2 (def_stmt);
       return (TREE_CODE (rhs1) == SSA_NAME && ssa_undefined_value_p (rhs1))
 	     || (TREE_CODE (rhs2) == SSA_NAME && ssa_undefined_value_p (rhs2));
     }
   return false;
@@ -1554,32 +1555,20 @@  execute_update_addresses_taken (void)
 		rhs = gimple_assign_rhs1 (stmt);
 		if (gimple_assign_lhs (stmt) != lhs
 		    && !useless_type_conversion_p (TREE_TYPE (lhs),
 						   TREE_TYPE (rhs)))
 		  rhs = fold_build1 (VIEW_CONVERT_EXPR,
 				     TREE_TYPE (lhs), rhs);
 
 		if (gimple_assign_lhs (stmt) != lhs)
 		  gimple_assign_set_lhs (stmt, lhs);
 
-		/* For var ={v} {CLOBBER}; where var lost
-		   TREE_ADDRESSABLE just remove the stmt.  */
-		if (DECL_P (lhs)
-		    && TREE_CLOBBER_P (rhs)
-		    && bitmap_bit_p (suitable_for_renaming, DECL_UID (lhs)))
-		  {
-		    unlink_stmt_vdef (stmt);
-      		    gsi_remove (&gsi, true);
-		    release_defs (stmt);
-		    continue;
-		  }
-
 		if (gimple_assign_rhs1 (stmt) != rhs)
 		  {
 		    gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
 		    gimple_assign_set_rhs_from_tree (&gsi, rhs);
 		  }
 	      }
 
 	    else if (gimple_code (stmt) == GIMPLE_CALL)
 	      {
 		unsigned i;
Index: gcc/tree-ssa.h
===================================================================
--- gcc/tree-ssa.h	(revision 217007)
+++ gcc/tree-ssa.h	(working copy)
@@ -44,21 +44,21 @@  extern tree target_for_debug_bind (tree)
 extern void insert_debug_temp_for_var_def (gimple_stmt_iterator *, tree);
 extern void insert_debug_temps_for_defs (gimple_stmt_iterator *);
 extern void reset_debug_uses (gimple);
 extern void release_defs_bitset (bitmap toremove);
 extern void verify_ssa (bool, bool);
 extern void init_tree_ssa (struct function *);
 extern void delete_tree_ssa (void);
 extern bool tree_ssa_useless_type_conversion (tree);
 extern tree tree_ssa_strip_useless_type_conversions (tree);
 
-extern bool ssa_undefined_value_p (tree);
+extern bool ssa_undefined_value_p (tree, bool = true);
 extern void execute_update_addresses_taken (void);
 
 /* Given an edge_var_map V, return the PHI arg definition.  */
 
 static inline tree
 redirect_edge_var_map_def (edge_var_map *v)
 {
   return v->def;
 }