On Fri, 31 Oct 2014, Richard Biener wrote:
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.
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.gli...@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 GlisseIndex: 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;
}
