On Sat, Oct 25, 2014 at 6:29 PM, Marc Glisse <marc.gli...@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.gli...@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.
> + {
> + /* 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.
> - 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.
Otherwise the patch looks ok.
Thanks for your patience.
Richard.
> + }
> + 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;
> Index: gcc/tree-ssa-live.c
> ===================================================================
> --- gcc/tree-ssa-live.c (revision 216689)
> +++ gcc/tree-ssa-live.c (working copy)
> @@ -40,20 +40,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
> @@ -1086,20 +1087,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
> @@ -1422,20 +1427,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 216689)
> +++ gcc/tree-ssa.c (working copy)
> @@ -1178,24 +1178,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
> @@ -1205,21 +1206,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;
> @@ -1551,32 +1552,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 216689)
> +++ 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;
> }
>
>
