This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PR48866] three alternative fixes
- From: Alexandre Oliva <aoliva at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Mon, 09 Apr 2012 03:08:10 -0300
- Subject: Re: [PR48866] three alternative fixes
- References: <ory61o1lpf.fsf@livre.localdomain> <ortycc1kdo.fsf@livre.localdomain> <or62onogs9.fsf@livre.localdomain>
On Jun 2, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
>> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
>>> I have 3 different, mutually exclusive patches that fix PR 48866. The
>>> problem is exponential time while dealing with an expression that
>>> resulted from a long chain of replaceable insns with memory accesses
>>> moved past the debug insns referring to their results.
>>> 1. emit debug temps for replaceable DEFs that end up being referenced in
>>> debug insns. We already have some code to try to deal with this, but it
>>> emits the huge expressions we'd rather avoid, and it may create
>>> unnecessary duplication. This new approach emits a placeholder instead
>>> of skipping replaceable DEFs altogether, and then, if the DEF is
>>> referenced in a debug insn (perhaps during the late debug re-expasion of
>>> some other placeholder), it is expanded. Placeholders that end up not
>>> being referenced are then throw away.
>> This is my favorite option, for it's safest: it doesn't change
>> executable code at all (or should I say it *shouldn't* change it, for I
>> haven't verified that it doesn't), retaining any register pressure
>> benefits from TER.
> This revised and retested version records expansions in an array indexed
> on SSA version rather than a pointer_map, as suggested by Matz.
Updated to deal with debug source bind stmts, added an assertion in
var-tracking to make sure we don't get unexpected kinds of decls in
VAR_LOCATION insns. Regstrapped on x86_64-linux-gnu and i686-linux-gnu.
Ok to install?
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/48866
* cfgexpand.c (DEBUG_INSN_TOEXPAND): New.
(def_expansions): New.
(def_expansions_init): New.
(def_expansions_remove_placeholder, def_expansions_fini): New.
(def_get_expansion_ptr): New.
(expand_debug_expr): Create debug temps as needed.
(expand_debug_insn): New, split out of...
(expand_debug_locations): ... this.
(gen_emit_debug_insn): New, split out of...
(expand_gimple_basic_block): ... this. Simplify expansion of
debug stmts. Emit placeholders for replaceable DEFs, rather
than debug temps at last non-debug uses.
(gimple_expand_cfg): Initialize and finalize expansions cache.
* var-tracking.c (use_type): Check for acceptable var decls in
var_locations.
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig 2012-04-08 01:43:59.334000124 -0300
+++ gcc/cfgexpand.c 2012-04-08 01:50:46.851123535 -0300
@@ -2611,6 +2611,70 @@ expand_debug_parm_decl (tree decl)
return NULL_RTX;
}
+/* Mark debug insns that are placeholders for replaceable SSA_NAMEs
+ that have not been expanded yet. */
+#define DEBUG_INSN_TOEXPAND(RTX) \
+ (RTL_FLAG_CHECK1("DEBUG_INSN_TOEXPAND", (RTX), DEBUG_INSN)->used)
+
+/* Map replaceable SSA_NAMEs versions to their RTL expansions. */
+static rtx *def_expansions;
+
+/* Initialize the def_expansions data structure. This is to be called
+ before expansion of a function starts. */
+
+static void
+def_expansions_init (void)
+{
+ gcc_checking_assert (!def_expansions);
+ def_expansions = XCNEWVEC (rtx, num_ssa_names);
+}
+
+/* Remove the DEBUG_INSN INSN if it still binds an SSA_NAME. */
+
+static bool
+def_expansions_remove_placeholder (rtx insn)
+{
+ gcc_checking_assert (insn);
+
+ if (TREE_CODE (INSN_VAR_LOCATION_DECL (insn)) == SSA_NAME)
+ {
+ gcc_assert (!DEBUG_INSN_TOEXPAND (insn));
+ remove_insn (insn);
+ }
+
+ return true;
+}
+
+/* Finalize the def_expansions data structure. This is to be called
+ at the end of the expansion of a function. */
+
+static void
+def_expansions_fini (void)
+{
+ int i = num_ssa_names;
+
+ gcc_checking_assert (def_expansions);
+ while (i--)
+ if (def_expansions[i])
+ def_expansions_remove_placeholder (def_expansions[i]);
+ XDELETEVEC (def_expansions);
+ def_expansions = NULL;
+}
+
+/* Return a pointer to the rtx expanded from EXP. EXP must be a
+ replaceable SSA_NAME. */
+
+static rtx *
+def_get_expansion_ptr (tree exp)
+{
+ gcc_checking_assert (def_expansions);
+ gcc_checking_assert (TREE_CODE (exp) == SSA_NAME);
+ gcc_checking_assert (bitmap_bit_p (SA.values, SSA_NAME_VERSION (exp)));
+ return &def_expansions[SSA_NAME_VERSION (exp)];
+}
+
+static void expand_debug_insn (rtx insn);
+
/* Return an RTX equivalent to the value of the tree expression EXP. */
static rtx
@@ -3421,7 +3485,30 @@ expand_debug_expr (tree exp)
gimple g = get_gimple_for_ssa_name (exp);
if (g)
{
- op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g));
+ rtx insn = *def_get_expansion_ptr (exp);
+ tree vexpr;
+
+ /* If this still has the original SSA_NAME, emit a debug
+ temp and compute the RTX value. */
+ if (TREE_CODE (INSN_VAR_LOCATION_DECL (insn)) == SSA_NAME)
+ {
+ tree var = SSA_NAME_VAR (INSN_VAR_LOCATION_DECL (insn));
+
+ vexpr = make_node (DEBUG_EXPR_DECL);
+ DECL_ARTIFICIAL (vexpr) = 1;
+ TREE_TYPE (vexpr) = TREE_TYPE (var);
+ DECL_MODE (vexpr) = DECL_MODE (var);
+ INSN_VAR_LOCATION_DECL (insn) = vexpr;
+
+ gcc_checking_assert (!DEBUG_INSN_TOEXPAND (insn));
+ DEBUG_INSN_TOEXPAND (insn) = 1;
+ expand_debug_insn (insn);
+ }
+ else
+ vexpr = INSN_VAR_LOCATION_DECL (insn);
+
+ op0 = expand_debug_expr (vexpr);
+
if (!op0)
return NULL;
}
@@ -3652,6 +3739,49 @@ expand_debug_source_expr (tree exp)
return op0;
}
+/* Expand the LOC value of the debug insn INSN. */
+
+static void
+expand_debug_insn (rtx insn)
+{
+ tree value = (tree)INSN_VAR_LOCATION_LOC (insn);
+ rtx val;
+ enum machine_mode mode;
+
+ gcc_checking_assert (TREE_CODE (INSN_VAR_LOCATION_DECL (insn)) != SSA_NAME);
+ gcc_checking_assert (DEBUG_INSN_TOEXPAND (insn));
+ DEBUG_INSN_TOEXPAND (insn) = 0;
+
+ if (value == NULL_TREE)
+ val = NULL_RTX;
+ else
+ {
+ rtx last = get_last_insn ();
+ if (INSN_VAR_LOCATION_STATUS (insn)
+ == VAR_INIT_STATUS_UNINITIALIZED)
+ val = expand_debug_source_expr (value);
+ else
+ val = expand_debug_expr (value);
+ gcc_checking_assert (last == get_last_insn ());
+ }
+
+ if (!val)
+ val = gen_rtx_UNKNOWN_VAR_LOC ();
+ else
+ {
+ mode = GET_MODE (INSN_VAR_LOCATION (insn));
+
+ gcc_assert (mode == GET_MODE (val)
+ || (GET_MODE (val) == VOIDmode
+ && (CONST_INT_P (val)
+ || GET_CODE (val) == CONST_FIXED
+ || GET_CODE (val) == CONST_DOUBLE
+ || GET_CODE (val) == LABEL_REF)));
+ }
+
+ INSN_VAR_LOCATION_LOC (insn) = val;
+}
+
/* Expand the _LOCs in debug insns. We run this after expanding all
regular insns, so that any variables referenced in the function
will have their DECL_RTLs set. */
@@ -3660,7 +3790,6 @@ static void
expand_debug_locations (void)
{
rtx insn;
- rtx last = get_last_insn ();
int save_strict_alias = flag_strict_aliasing;
/* New alias sets while setting up memory attributes cause
@@ -3669,42 +3798,54 @@ expand_debug_locations (void)
flag_strict_aliasing = 0;
for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
- if (DEBUG_INSN_P (insn))
- {
- tree value = (tree)INSN_VAR_LOCATION_LOC (insn);
- rtx val;
- enum machine_mode mode;
+ /* Skip debug insns that have SSA_NAMEs as bound decls. These
+ will only have their values expanded if referenced by other
+ debug insns, and they are removed if not expanded. */
+ if (DEBUG_INSN_P (insn) && DEBUG_INSN_TOEXPAND (insn))
+ expand_debug_insn (insn);
- if (value == NULL_TREE)
- val = NULL_RTX;
- else
- {
- if (INSN_VAR_LOCATION_STATUS (insn)
- == VAR_INIT_STATUS_UNINITIALIZED)
- val = expand_debug_source_expr (value);
- else
- val = expand_debug_expr (value);
- gcc_assert (last == get_last_insn ());
- }
+ flag_strict_aliasing = save_strict_alias;
+}
- if (!val)
- val = gen_rtx_UNKNOWN_VAR_LOC ();
- else
- {
- mode = GET_MODE (INSN_VAR_LOCATION (insn));
+/* Emit a debug insn that binds VAR to VALUE, with location and block
+ information taken from STMT. */
- gcc_assert (mode == GET_MODE (val)
- || (GET_MODE (val) == VOIDmode
- && (CONST_INT_P (val)
- || GET_CODE (val) == CONST_FIXED
- || GET_CODE (val) == CONST_DOUBLE
- || GET_CODE (val) == LABEL_REF)));
- }
+static rtx
+gen_emit_debug_insn (tree var, tree value, gimple stmt)
+{
+ enum machine_mode mode;
+ location_t sloc = get_curr_insn_source_location ();
+ tree sblock = get_curr_insn_block ();
+ rtx val, insn;
+
+ if (DECL_P (var))
+ mode = DECL_MODE (var);
+ else if (TREE_CODE (var) == SSA_NAME)
+ mode = DECL_MODE (SSA_NAME_VAR (var));
+ else
+ mode = TYPE_MODE (TREE_TYPE (var));
- INSN_VAR_LOCATION_LOC (insn) = val;
- }
+ val = gen_rtx_VAR_LOCATION
+ (mode, var, (rtx)value, VAR_INIT_STATUS_INITIALIZED);
- flag_strict_aliasing = save_strict_alias;
+ set_curr_insn_source_location (gimple_location (stmt));
+ set_curr_insn_block (gimple_block (stmt));
+ insn = emit_debug_insn (val);
+ set_curr_insn_source_location (sloc);
+ set_curr_insn_block (sblock);
+
+ DEBUG_INSN_TOEXPAND (insn) = (TREE_CODE (var) != SSA_NAME);
+
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ /* We can't dump the insn with a TREE where an RTX
+ is expected. */
+ PAT_VAR_LOCATION_LOC (val) = const0_rtx;
+ maybe_dump_rtl_for_gimple_stmt (stmt, PREV_INSN (insn));
+ PAT_VAR_LOCATION_LOC (val) = (rtx)value;
+ }
+
+ return insn;
}
/* Expand basic block BB from GIMPLE trees to RTL. */
@@ -3796,104 +3937,6 @@ expand_gimple_basic_block (basic_block b
stmt = gsi_stmt (gsi);
- /* If this statement is a non-debug one, and we generate debug
- insns, then this one might be the last real use of a TERed
- SSA_NAME, but where there are still some debug uses further
- down. Expanding the current SSA name in such further debug
- uses by their RHS might lead to wrong debug info, as coalescing
- might make the operands of such RHS be placed into the same
- pseudo as something else. Like so:
- a_1 = a_0 + 1; // Assume a_1 is TERed and a_0 is dead
- use(a_1);
- a_2 = ...
- #DEBUG ... => a_1
- As a_0 and a_2 don't overlap in lifetime, assume they are coalesced.
- If we now would expand a_1 by it's RHS (a_0 + 1) in the debug use,
- the write to a_2 would actually have clobbered the place which
- formerly held a_0.
-
- So, instead of that, we recognize the situation, and generate
- debug temporaries at the last real use of TERed SSA names:
- a_1 = a_0 + 1;
- #DEBUG #D1 => a_1
- use(a_1);
- a_2 = ...
- #DEBUG ... => #D1
- */
- if (MAY_HAVE_DEBUG_INSNS
- && SA.values
- && !is_gimple_debug (stmt))
- {
- ssa_op_iter iter;
- tree op;
- gimple def;
-
- location_t sloc = get_curr_insn_source_location ();
- tree sblock = get_curr_insn_block ();
-
- /* Look for SSA names that have their last use here (TERed
- names always have only one real use). */
- FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_USE)
- if ((def = get_gimple_for_ssa_name (op)))
- {
- imm_use_iterator imm_iter;
- use_operand_p use_p;
- bool have_debug_uses = false;
-
- FOR_EACH_IMM_USE_FAST (use_p, imm_iter, op)
- {
- if (gimple_debug_bind_p (USE_STMT (use_p)))
- {
- have_debug_uses = true;
- break;
- }
- }
-
- if (have_debug_uses)
- {
- /* OP is a TERed SSA name, with DEF it's defining
- statement, and where OP is used in further debug
- instructions. Generate a debug temporary, and
- replace all uses of OP in debug insns with that
- temporary. */
- gimple debugstmt;
- tree value = gimple_assign_rhs_to_tree (def);
- tree vexpr = make_node (DEBUG_EXPR_DECL);
- rtx val;
- enum machine_mode mode;
-
- set_curr_insn_source_location (gimple_location (def));
- set_curr_insn_block (gimple_block (def));
-
- DECL_ARTIFICIAL (vexpr) = 1;
- TREE_TYPE (vexpr) = TREE_TYPE (value);
- if (DECL_P (value))
- mode = DECL_MODE (value);
- else
- mode = TYPE_MODE (TREE_TYPE (value));
- DECL_MODE (vexpr) = mode;
-
- val = gen_rtx_VAR_LOCATION
- (mode, vexpr, (rtx)value, VAR_INIT_STATUS_INITIALIZED);
-
- emit_debug_insn (val);
-
- FOR_EACH_IMM_USE_STMT (debugstmt, imm_iter, op)
- {
- if (!gimple_debug_bind_p (debugstmt))
- continue;
-
- FOR_EACH_IMM_USE_ON_STMT (use_p, imm_iter)
- SET_USE (use_p, vexpr);
-
- update_stmt (debugstmt);
- }
- }
- }
- set_curr_insn_source_location (sloc);
- set_curr_insn_block (sblock);
- }
-
currently_expanding_gimple_stmt = stmt;
/* Expand this statement, then evaluate the resulting RTL and
@@ -3906,103 +3949,42 @@ expand_gimple_basic_block (basic_block b
}
else if (gimple_debug_bind_p (stmt))
{
- location_t sloc = get_curr_insn_source_location ();
- tree sblock = get_curr_insn_block ();
- gimple_stmt_iterator nsi = gsi;
-
- for (;;)
- {
- tree var = gimple_debug_bind_get_var (stmt);
- tree value;
- rtx val;
- enum machine_mode mode;
-
- if (TREE_CODE (var) != DEBUG_EXPR_DECL
- && TREE_CODE (var) != LABEL_DECL
- && !target_for_debug_bind (var))
- goto delink_debug_stmt;
-
- if (gimple_debug_bind_has_value_p (stmt))
- value = gimple_debug_bind_get_value (stmt);
- else
- value = NULL_TREE;
-
- last = get_last_insn ();
-
- set_curr_insn_source_location (gimple_location (stmt));
- set_curr_insn_block (gimple_block (stmt));
-
- if (DECL_P (var))
- mode = DECL_MODE (var);
- else
- mode = TYPE_MODE (TREE_TYPE (var));
-
- val = gen_rtx_VAR_LOCATION
- (mode, var, (rtx)value, VAR_INIT_STATUS_INITIALIZED);
-
- emit_debug_insn (val);
+ tree var = gimple_debug_bind_get_var (stmt);
+ tree value;
- if (dump_file && (dump_flags & TDF_DETAILS))
- {
- /* We can't dump the insn with a TREE where an RTX
- is expected. */
- PAT_VAR_LOCATION_LOC (val) = const0_rtx;
- maybe_dump_rtl_for_gimple_stmt (stmt, last);
- PAT_VAR_LOCATION_LOC (val) = (rtx)value;
- }
+ /* We may tentatively emit debug stmts for non-SSA
+ variables, in the hope they become gimple regs, but if
+ they don't, at this point we have to discard them. We
+ might actually emit a debug insn, and arrange for
+ var-tracking to use a MO_VAL_SET to introduce the
+ binding, but the semantics of dataflow confluence for
+ NOT_ONEPART variables is different, and expressions bound
+ to them are not supposed to contain VALUEs, so this would
+ most likely require further work. Drop the annotation on
+ the floor for now. */
+ if (TREE_CODE (var) != DEBUG_EXPR_DECL
+ && TREE_CODE (var) != LABEL_DECL
+ && !target_for_debug_bind (var))
+ continue;
- delink_debug_stmt:
- /* In order not to generate too many debug temporaries,
- we delink all uses of debug statements we already expanded.
- Therefore debug statements between definition and real
- use of TERed SSA names will continue to use the SSA name,
- and not be replaced with debug temps. */
- delink_stmt_imm_use (stmt);
-
- gsi = nsi;
- gsi_next (&nsi);
- if (gsi_end_p (nsi))
- break;
- stmt = gsi_stmt (nsi);
- if (!gimple_debug_bind_p (stmt))
- break;
- }
+ if (gimple_debug_bind_has_value_p (stmt))
+ value = gimple_debug_bind_get_value (stmt);
+ else
+ value = NULL_TREE;
- set_curr_insn_source_location (sloc);
- set_curr_insn_block (sblock);
+ gen_emit_debug_insn (var, value, stmt);
}
else if (gimple_debug_source_bind_p (stmt))
{
- location_t sloc = get_curr_insn_source_location ();
- tree sblock = get_curr_insn_block ();
tree var = gimple_debug_source_bind_get_var (stmt);
tree value = gimple_debug_source_bind_get_value (stmt);
- rtx val;
- enum machine_mode mode;
-
- last = get_last_insn ();
-
- set_curr_insn_source_location (gimple_location (stmt));
- set_curr_insn_block (gimple_block (stmt));
-
- mode = DECL_MODE (var);
- val = gen_rtx_VAR_LOCATION (mode, var, (rtx)value,
- VAR_INIT_STATUS_UNINITIALIZED);
+ gcc_checking_assert
+ (!(TREE_CODE (var) != DEBUG_EXPR_DECL
+ && TREE_CODE (var) != LABEL_DECL
+ && !target_for_debug_bind (var)));
- emit_debug_insn (val);
-
- if (dump_file && (dump_flags & TDF_DETAILS))
- {
- /* We can't dump the insn with a TREE where an RTX
- is expected. */
- PAT_VAR_LOCATION_LOC (val) = const0_rtx;
- maybe_dump_rtl_for_gimple_stmt (stmt, last);
- PAT_VAR_LOCATION_LOC (val) = (rtx)value;
- }
-
- set_curr_insn_source_location (sloc);
- set_curr_insn_block (sblock);
+ gen_emit_debug_insn (var, value, stmt);
}
else
{
@@ -4023,14 +4005,29 @@ expand_gimple_basic_block (basic_block b
def_operand_p def_p;
def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
- if (def_p != NULL)
+ /* If this stmt is in the list of replaceable
+ expressions, expand a placeholder for a debug insn. */
+ if (def_p != NULL && SA.values
+ && bitmap_bit_p (SA.values,
+ SSA_NAME_VERSION (DEF_FROM_PTR (def_p))))
{
- /* Ignore this stmt if it is in the list of
- replaceable expressions. */
- if (SA.values
- && bitmap_bit_p (SA.values,
- SSA_NAME_VERSION (DEF_FROM_PTR (def_p))))
+ tree def = DEF_FROM_PTR (def_p);
+ rtx *xp;
+
+ gcc_checking_assert (TREE_CODE (def) == SSA_NAME
+ && DECL_P (SSA_NAME_VAR (def)));
+
+ if (!MAY_HAVE_DEBUG_INSNS)
continue;
+
+ xp = def_get_expansion_ptr (def);
+ gcc_checking_assert (!*xp);
+ *xp = last
+ = gen_emit_debug_insn (def,
+ gimple_assign_rhs_to_tree (stmt),
+ stmt);
+
+ continue;
}
last = expand_gimple_stmt (stmt);
maybe_dump_rtl_for_gimple_stmt (stmt, last);
@@ -4549,11 +4546,18 @@ gimple_expand_cfg (void)
e->flags &= ~EDGE_EXECUTABLE;
lab_rtx_for_bb = pointer_map_create ();
+
+ if (MAY_HAVE_DEBUG_INSNS)
+ def_expansions_init ();
+
FOR_BB_BETWEEN (bb, init_block->next_bb, EXIT_BLOCK_PTR, next_bb)
bb = expand_gimple_basic_block (bb);
if (MAY_HAVE_DEBUG_INSNS)
- expand_debug_locations ();
+ {
+ expand_debug_locations ();
+ def_expansions_fini ();
+ }
execute_free_datastructures ();
timevar_push (TV_OUT_OF_SSA);
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig 2012-04-08 01:50:25.657377155 -0300
+++ gcc/var-tracking.c 2012-04-08 01:50:46.930122589 -0300
@@ -4967,7 +4967,16 @@ use_type (rtx loc, struct count_use_info
{
if (GET_CODE (loc) == VAR_LOCATION)
{
- if (track_expr_p (PAT_VAR_LOCATION_DECL (loc), false))
+ tree var = PAT_VAR_LOCATION_DECL (loc);
+
+ /* This is consistent with what annotations that we emit in
+ cfgexpand; those for which the assertion doesn't hold
+ should be dropped on the floor at that point, rather than
+ being carried over. */
+ gcc_checking_assert (TREE_CODE (var) == DEBUG_EXPR_DECL
+ || TREE_CODE (var) == LABEL_DECL
+ || target_for_debug_bind (var));
+ if (track_expr_p (var, false))
{
rtx ploc = PAT_VAR_LOCATION_LOC (loc);
if (! VAR_LOC_UNKNOWN_P (ploc))
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist Red Hat Brazil Compiler Engineer