* [PR48866] three alternative fixes
@ 2011-05-30 13:25 Alexandre Oliva
2011-05-30 13:34 ` Alexandre Oliva
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Alexandre Oliva @ 2011-05-30 13:25 UTC (permalink / raw)
To: gcc-patches
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.
2. emit placeholders for replaceable DEFs and, when the DEFs are
expanded at their point of use, emit the expansion next to the
placeholder, rather than at the current stream. The result of the
expansion is saved and used in debug insns that reference the
replaceable DEF. If the result is forced into a REG shortly thereafter,
the code resulting from this is also emitted next to the placeholder,
and the saved expansion is updated. If the USE is expanded before the
DEF, the insn stream resulting from the expansion is saved and emitted
at the point of the DEF.
3. expand dominators before dominated blocks, so that DEFs of
replaceable SSA names are expanded before their uses. Expand them when
they're encountered, but not requiring a REG as a result. Save the RTL
expression that results from the expansion for use in debug insns and at
the non-debug use.
I'll post each patch with further details in separate follow-up e-mails.
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2011-05-30 13:25 [PR48866] three alternative fixes Alexandre Oliva
@ 2011-05-30 13:34 ` Alexandre Oliva
2011-06-03 1:21 ` Alexandre Oliva
2011-05-30 14:30 ` Alexandre Oliva
2011-05-30 14:55 ` Alexandre Oliva
2 siblings, 1 reply; 12+ messages in thread
From: Alexandre Oliva @ 2011-05-30 13:34 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2674 bytes --]
On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
> 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.
The drawbacks are higher likelihood of loss of debug information as
complex debug value expressions now require the recursive expansion of
multiple debug temps, which is now more likely to hit the recursion
limit. E.g., for a chain of dereferences, before the patch we'd get
something like:
;; x_ = ****y_; (actually a sequence of replaceable gimple assignments)
;; debug x => x_
(debug_insn (var_location x (mem (mem (mem (mem (reg y_)))))))
[...]
;; ? = x_ + 1;
(set (reg) (mem (reg y_)))
(set (reg) (mem (reg...)))
(set (reg) (mem (reg...)))
(set (reg x_) (mem (reg...)))
(set (reg ?) (plus (reg x_) (const_int 1)))
After the patch, the first two stmts would expand to:
;; x_ = ****y; (replaceable sequence)
(debug_insn (var_location D#3 (mem (reg ...))))
(debug_insn (var_location D#2 (mem D#3)))
(debug_insn (var_location D#1 (mem D#2)))
;; debug x => x_
(debug_insn (var_location x (mem D#1)))
The additional debug temps will likely come at a compile-time cost,
especially during var-tracking, but this may be at least in part
recovered because the debug temps hold values that are likely to be
computed by actual insns, so var-tracking will create value tracking
entries for them anyway.
Unlike the other patches, this alternative will *not* expand
replaceable code at its natural location, so the executable code will
remain at the point of use. Single-stepping will therefore remain just
as confusing, and since debug bind stmts will likely remain at points
one can't stop, they may remain unusable. The debug information
extensions I proposed at the GCC Summit last year will alleviate this
issue, but I still don't have a concrete plan for completing the
implementation of that feature, so this is something to keep in mind
if this patch is preferred.
Here's the patch. Regstrapped on x86_64-linux-gnu and i686-linux-gnu.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: expand-replaceable-in-order-pr48866.patch --]
[-- Type: text/x-diff, Size: 15369 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/48866
* cfgexpand.c (def_expansions): New.
(def_expansions_init): New.
(def_expansions_remove_placeholder, def_expansions_fini): New.
(def_has_expansion_ptr, 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.
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig 2011-05-28 08:40:42.000000000 -0300
+++ gcc/cfgexpand.c 2011-05-28 11:33:22.727418563 -0300
@@ -2337,6 +2337,84 @@ convert_debug_memory_address (enum machi
return x;
}
+/* Map replaceable SSA_NAMEs to their RTL expansions. */
+static struct pointer_map_t *def_expansions;
+
+/* 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)
+
+/* 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 = pointer_map_create ();
+}
+
+/* Remove the DEBUG_INSN at **XP if it still binds an SSA_NAME. */
+
+static bool
+def_expansions_remove_placeholder (const void *def ATTRIBUTE_UNUSED,
+ void **xp,
+ void *arg ATTRIBUTE_UNUSED)
+{
+ rtx insn = (rtx) *xp;
+
+ 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)
+{
+ gcc_checking_assert (def_expansions);
+ pointer_map_traverse (def_expansions,
+ def_expansions_remove_placeholder, NULL);
+ pointer_map_destroy (def_expansions);
+ def_expansions = NULL;
+}
+
+/* Return NULL if no expansion is registered for EXP, or a pointer to
+ the rtx expanded from it otherwise. EXP must be a replaceable
+ SSA_NAME. */
+
+static void *const *
+def_has_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 pointer_map_contains (def_expansions, exp);
+}
+
+/* Return a pointer to the rtx expanded from EXP. EXP must be a
+ replaceable SSA_NAME. */
+
+static void **
+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 pointer_map_insert (def_expansions, exp);
+}
+
+static void expand_debug_insn (rtx insn);
+
/* Return an RTX equivalent to the value of the tree expression
EXP. */
@@ -3131,7 +3209,35 @@ 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));
+ void *const *xp = def_has_expansion_ptr (exp);
+ rtx insn;
+ tree vexpr;
+
+ gcc_checking_assert (xp);
+
+ insn = (rtx) *xp;
+
+ /* 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;
}
@@ -3293,6 +3399,45 @@ expand_debug_expr (tree exp)
}
}
+/* 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 ();
+ 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. */
@@ -3301,7 +3446,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
@@ -3310,38 +3454,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
- {
- 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. */
@@ -3433,104 +3593,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
@@ -3543,64 +3605,15 @@ 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 (gimple_debug_bind_has_value_p (stmt))
- value = gimple_debug_bind_get_value (stmt);
- else
- value = NULL_TREE;
+ tree var = gimple_debug_bind_get_var (stmt);
+ tree value;
- 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);
-
- 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;
- }
-
- /* 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
{
@@ -3621,14 +3634,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);
+ void **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);
@@ -3970,6 +3998,7 @@ gimple_expand_cfg (void)
rtx var_seq;
unsigned i;
+
timevar_push (TV_OUT_OF_SSA);
rewrite_out_of_ssa (&SA);
timevar_pop (TV_OUT_OF_SSA);
@@ -4112,11 +4141,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);
[-- Attachment #3: Type: text/plain, Size: 258 bytes --]
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2011-05-30 13:25 [PR48866] three alternative fixes Alexandre Oliva
2011-05-30 13:34 ` Alexandre Oliva
@ 2011-05-30 14:30 ` Alexandre Oliva
2011-06-03 1:28 ` Alexandre Oliva
2011-06-03 1:37 ` Alexandre Oliva
2011-05-30 14:55 ` Alexandre Oliva
2 siblings, 2 replies; 12+ messages in thread
From: Alexandre Oliva @ 2011-05-30 14:30 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 3058 bytes --]
On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
> 2. emit placeholders for replaceable DEFs and, when the DEFs are
> expanded at their point of use, emit the expansion next to the
> placeholder, rather than at the current stream. The result of the
> expansion is saved and used in debug insns that reference the
> replaceable DEF. If the result is forced into a REG shortly thereafter,
> the code resulting from this is also emitted next to the placeholder,
> and the saved expansion is updated. If the USE is expanded before the
> DEF, the insn stream resulting from the expansion is saved and emitted
> at the point of the DEF.
IMHO this is the riskiest of the 3 patches, for shuffling expansions
around isn't exactly something I'm comfortable with. There's a very
real risk that moving the expansion of sub-expressions to their
definition points may end up moving uses before definitions.
I've actually caught a few such cases, and the checks on whether the
result of expanding the sub-expression is the requested target, the
handling of recent expansions and the resetting of recent expansions
across basic blocks were all in reponse to such cases. I'm not sure
there aren't other, so I'm not entirely comfortable with this approach.
However, it survived regstrap on x86_64-linux-gnu and i686-linux-gnu, at
various optimization levels, so it might actually be safe.
One nice property of this patch is that debug insns remain after the
expansion of DEFs they refer to, so they will most often bind to a
simple expression, even a REG, if the expansion was forced to one. This
is superior to the two other patches in that users of a debugger will be
able to modify the user variable, whereas with the others, the variable
will likely be bound to a computed expression, which the debugger will
be unable to modify.
Of course, moving replaceable expressions to their original points
extends the live ranges of their results, be they registers or computed
expressions involving other registers.
With this patch, the generated code looks like this:
;; x_ = ****y_; (actually a sequence of replaceable gimple assignments)
(set (reg) (mem (reg y_)))
(set (reg) (mem (reg...)))
(set (reg) (mem (reg...)))
(set (reg x_) (mem (reg...)))
;; debug x => x_
(debug_insn (var_location x (reg x_)))
[...]
;; ? = x_ + 1;
(set (reg ?) (plus (reg x_) (const_int 1)))
Another negative point of this patch is that it emits placeholder notes,
that are discarded at the end of expansion, i.e., garbage. One
complication is that, since DEF and USE may be in different blocks, and
there's no guarantee that DEF will be expanded before USE, we end up
having to support the possibility that the USE is expanded first. In
either case, the insns are emitted in a separate sequence, and then
emitted at the proper place, which is an additional inefficiency at
expand time. Some of this may be recovered by the simpler debug
expressions, if they survive, but only for -g compilations.
Here's the patch, regstrapped on x86_64-linux-gnu and i686-linux-gnu.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: expand-replaceable-in-order-pr48866-3.patch --]
[-- Type: text/x-diff, Size: 14554 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/48866
* cfgexpand.c (def_expansions): New.
(def_expansion_recent_tree, def_expansion_recent_rtx): New.
(def_expansions_init): New.
(def_expansions_remove_placeholder, def_expansions_fini): New.
(def_has_expansion_ptr, def_get_expansion_ptr): New.
(def_expansion_recent, def_expansion_record_recent): New.
(def_expansion_add_insns): New.
(expand_debug_expr): Use recorded expansion if available.
(expand_gimple_basic_block): Emit or prepare to record
expansion of replaceable defs. Reset recent expansions at the
end of the block.
(gimple_expand_cfg): Initialize and finalize expansions cache.
* expr.c: Include gimple-pretty-print.h.
(store_expr): Forget recent expansions upon nontemporal moves.
(expand_expr_real_1): Reuse or record expansion of replaceable
defs.
* expr.h (def_get_expansion_ptr, def_expansion_recent): Declare.
(def_expansion_record_recent, def_expansion_add_insns): Declare.
* explow.c (force_recent): New.
(force_reg): Use it. Split into...
(force_reg_1): ... this.
* Makefile.in (expr.o): Depend on gimple-pretty-print.h.
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig 2011-05-26 05:03:08.492988988 -0300
+++ gcc/cfgexpand.c 2011-05-26 05:03:31.861035336 -0300
@@ -2337,6 +2337,166 @@ convert_debug_memory_address (enum machi
return x;
}
+/* Map replaceable SSA_NAMEs to NOTE_INSN_VAR_LOCATIONs that hold
+ their RTL expansions (once available) in their NOTE_VAR_LOCATIONs
+ (without a VAR_LOCATION rtx). The NOTE is always after the
+ expansion insns.
+
+ If the SSA_NAME DEF is expanded before its single USE, the NOTE is
+ inserted in the insn stream, marking the location where the
+ non-replaceable portion of the expansion is to be inserted.
+
+ If the USE is expanded first, the NOTE and the expansion are
+ emitted as a separate insn chain, that is to be re-emitted when the
+ DEF is expanded. */
+static struct pointer_map_t *def_expansions;
+
+/* The latest expanded SSA name, and its corresponding RTL expansion.
+ These are used to enable the insertion of the insn that stores the
+ expansion in a register at the end of the sequence expanded for the
+ SSA DEF. */
+static tree def_expansion_recent_tree;
+static rtx def_expansion_recent_rtx;
+
+/* 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 = pointer_map_create ();
+
+ gcc_checking_assert (!def_expansion_recent_tree);
+ gcc_checking_assert (!def_expansion_recent_rtx);
+}
+
+/* Remove the NOTE at **XP that marks the insertion location of the
+ expansion of a replaceable SSA note. */
+
+static bool
+def_expansions_remove_placeholder (const void *def ATTRIBUTE_UNUSED,
+ void **xp,
+ void *arg ATTRIBUTE_UNUSED)
+{
+ rtx note = (rtx) *xp;
+
+ if (!note)
+ return true;
+
+ gcc_checking_assert (NOTE_P (note));
+ remove_insn (note);
+
+ 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)
+{
+ gcc_checking_assert (def_expansions);
+
+ pointer_map_traverse (def_expansions,
+ def_expansions_remove_placeholder, NULL);
+
+ pointer_map_destroy (def_expansions);
+ def_expansions = NULL;
+ def_expansion_recent_tree = NULL;
+ def_expansion_recent_rtx = NULL;
+}
+
+/* Return NULL if no expansions is registered for EXP, or a pointer to
+ the address of its location NOTE otherwise. EXP must be a
+ replaceable SSA_NAME. */
+
+static void *const *
+def_has_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 pointer_map_contains (def_expansions, exp);
+}
+
+/* Return a pointer to the address of EXP's location NOTE. If the
+ address (not the pointer) is NULL, the caller must initialize it
+ with a newly-created NOTE. EXP must be a replaceable SSA_NAME. */
+
+void **
+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 pointer_map_insert (def_expansions, exp);
+}
+
+/* Return an SSA name, if any, that was recently expanded to the value
+ X. */
+
+tree
+def_expansion_recent (rtx x)
+{
+ if (!def_expansion_recent_rtx)
+ return NULL;
+
+ if (x == def_expansion_recent_rtx
+ || rtx_equal_p (x, def_expansion_recent_rtx))
+ return def_expansion_recent_tree;
+
+ return NULL;
+}
+
+/* Record that DEF was recently expanded to the value X. */
+
+void
+def_expansion_record_recent (tree def, rtx x)
+{
+ def_expansion_recent_tree = def;
+ def_expansion_recent_rtx = x;
+}
+
+/* Forget recent expansions. */
+
+void
+def_expansion_reset_recent (void)
+{
+ def_expansion_record_recent (NULL, NULL);
+}
+
+/* Add INSNS to the insn seq generated for DEF, and update the
+ RTL value of DEF to VAL. */
+
+void
+def_expansion_add_insns (tree def, rtx insns, rtx val)
+{
+ void **xp = def_get_expansion_ptr (def);
+ rtx note = (rtx) *xp;
+
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "\n;; (cont) ");
+ print_gimple_stmt (dump_file, SSA_NAME_DEF_STMT (def), 0,
+ TDF_SLIM | (dump_flags & TDF_LINENO));
+ fprintf (dump_file, "\n");
+
+ print_rtl (dump_file, insns);
+
+ fprintf (dump_file, "\n=> ");
+ print_rtl (dump_file, val);
+ }
+
+ gcc_checking_assert (note);
+ emit_insn_before (insns, note);
+ gcc_checking_assert (NOTE_VAR_LOCATION (note));
+ NOTE_VAR_LOCATION (note) = val;
+
+ def_expansion_recent_tree = NULL;
+ def_expansion_recent_rtx = NULL;
+}
+
/* Return an RTX equivalent to the value of the tree expression
EXP. */
@@ -3131,7 +3291,20 @@ 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));
+ void *const *xp = def_has_expansion_ptr (exp);
+
+ if (xp)
+ {
+ rtx note = (rtx) *xp;
+ gcc_checking_assert (NOTE_P (note));
+ op0 = NOTE_VAR_LOCATION (note);
+ }
+ else
+ op0 = NULL;
+
+ if (!op0)
+ op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g));
+
if (!op0)
return NULL;
}
@@ -3621,22 +3794,46 @@ 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)
+ /* Ignore this stmt if it is in the list of
+ replaceable expressions. */
+ 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))))
- continue;
+ tree def = DEF_FROM_PTR (def_p);
+ void **xp = def_get_expansion_ptr (def);
+
+ last = get_last_insn ();
+
+ if (!*xp)
+ {
+ rtx note = emit_note (NOTE_INSN_VAR_LOCATION);
+ NOTE_VAR_LOCATION (note) = NULL;
+ *xp = note;
+ }
+ else
+ {
+ rtx note = (rtx) *xp;
+ rtx first;
+
+ for (first = note;
+ PREV_INSN (first);
+ first = PREV_INSN (first))
+ ;
+
+ emit_insn (first);
+ }
}
- last = expand_gimple_stmt (stmt);
+ else
+ last = expand_gimple_stmt (stmt);
maybe_dump_rtl_for_gimple_stmt (stmt, last);
}
}
}
currently_expanding_gimple_stmt = NULL;
+ def_expansion_reset_recent ();
/* Expand implicit goto and convert goto_locus. */
FOR_EACH_EDGE (e, ei, bb->succs)
@@ -4112,11 +4309,14 @@ gimple_expand_cfg (void)
e->flags &= ~EDGE_EXECUTABLE;
lab_rtx_for_bb = pointer_map_create ();
+ 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 ();
+ def_expansions_fini ();
execute_free_datastructures ();
timevar_push (TV_OUT_OF_SSA);
Index: gcc/expr.c
===================================================================
--- gcc/expr.c.orig 2011-05-26 05:03:08.493988990 -0300
+++ gcc/expr.c 2011-05-26 05:03:31.869035352 -0300
@@ -48,6 +48,7 @@ along with GCC; see the file COPYING3.
#include "tree-iterator.h"
#include "tree-pass.h"
#include "tree-flow.h"
+#include "gimple-pretty-print.h"
#include "target.h"
#include "timevar.h"
#include "df.h"
@@ -4675,6 +4676,9 @@ store_expr (tree exp, rtx target, int ca
(call_param_p
? EXPAND_STACK_PARM : EXPAND_NORMAL),
&alt_rtl);
+ /* Don't risk moving loads before stores. */
+ if (!nontemporal)
+ def_expansion_reset_recent ();
}
/* If TEMP is a VOIDmode constant and the mode of the type of EXP is not
@@ -8424,10 +8428,71 @@ expand_expr_real_1 (tree exp, rtx target
&& !SSA_NAME_IS_DEFAULT_DEF (exp)
&& (optimize || DECL_IGNORED_P (SSA_NAME_VAR (exp)))
&& stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp)))
- g = SSA_NAME_DEF_STMT (exp);
+ {
+ g = SSA_NAME_DEF_STMT (exp);
+ if (g)
+ return expand_expr_real (gimple_assign_rhs_to_tree (g),
+ target, tmode, modifier, NULL);
+ }
if (g)
- return expand_expr_real (gimple_assign_rhs_to_tree (g), target, tmode,
- modifier, NULL);
+ {
+ rtx retval;
+ rtx insns;
+ void **xp = def_get_expansion_ptr (exp);
+ rtx note = (rtx) *xp;
+ bool def_expanded = !!note;
+
+ if (note && NOTE_VAR_LOCATION (note))
+ return NOTE_VAR_LOCATION (note);
+
+ start_sequence ();
+
+ retval = expand_expr_real (gimple_assign_rhs_to_tree (g),
+ target, tmode, modifier, NULL);
+
+ insns = get_insns ();
+
+ if (retval == target)
+ {
+ end_sequence ();
+ emit_insn (insns);
+ return retval;
+ }
+
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "\n;; ");
+ print_gimple_stmt (dump_file, g, 0,
+ TDF_SLIM | (dump_flags & TDF_LINENO));
+ fprintf (dump_file, "\n");
+
+ print_rtl (dump_file, insns);
+
+ fprintf (dump_file, "\n=> ");
+ print_rtl (dump_file, retval);
+ }
+
+ if (!def_expanded)
+ {
+ note = emit_note (NOTE_INSN_VAR_LOCATION);
+ *xp = note;
+ }
+
+ end_sequence ();
+
+ if (def_expanded)
+ {
+ /* Make sure this is not in the instruction stream. */
+ gcc_checking_assert (PREV_INSN (note));
+ emit_insn_before (insns, note);
+ gcc_checking_assert (!NOTE_VAR_LOCATION (note));
+ }
+
+ NOTE_VAR_LOCATION (note) = retval;
+ def_expansion_record_recent (exp, retval);
+
+ return retval;
+ }
ssa_name = exp;
decl_rtl = get_rtx_for_ssa_name (ssa_name);
Index: gcc/expr.h
===================================================================
--- gcc/expr.h.orig 2011-05-26 05:03:08.494988992 -0300
+++ gcc/expr.h 2011-05-26 05:03:31.897035408 -0300
@@ -693,4 +693,11 @@ extern tree build_libfunc_function (cons
/* Get the personality libfunc for a function decl. */
rtx get_personality_function (tree);
+/* In cfgexpand.c. */
+void **def_get_expansion_ptr (tree);
+tree def_expansion_recent (rtx);
+void def_expansion_record_recent (tree, rtx);
+void def_expansion_reset_recent (void);
+void def_expansion_add_insns (tree, rtx, rtx);
+
#endif /* GCC_EXPR_H */
Index: gcc/explow.c
===================================================================
--- gcc/explow.c.orig 2011-05-26 05:03:08.495988994 -0300
+++ gcc/explow.c 2011-05-26 05:03:31.900035414 -0300
@@ -638,22 +638,62 @@ copy_to_mode_reg (enum machine_mode mode
return temp;
}
+/* If X is the value of a recently-expanded SSA def, emit the insns
+ generated by FN (MODE, X) at the end of the expansion of that def,
+ otherwise emit them in the current insn seq. */
+
+static inline rtx
+force_recent (enum machine_mode mode, rtx x,
+ rtx (*fn) (enum machine_mode, rtx))
+{
+ tree def = def_expansion_recent (x);
+ rtx retval;
+ rtx insns;
+
+ if (!def)
+ return fn (mode, x);
+
+ start_sequence ();
+ retval = fn (mode, x);
+ insns = get_insns ();
+ end_sequence ();
+
+ def_expansion_add_insns (def, insns, retval);
+
+ return retval;
+}
+
+static rtx force_reg_1 (enum machine_mode, rtx);
+
/* Load X into a register if it is not already one.
Use mode MODE for the register.
X should be valid for mode MODE, but it may be a constant which
is valid for all integer modes; that's why caller must specify MODE.
+ If X is the value of a recent SSA def expansion, emit the insns at
+ the end of that expansion, rather than into the current seq.
+
The caller must not alter the value in the register we return,
since we mark it as a "constant" register. */
rtx
force_reg (enum machine_mode mode, rtx x)
{
- rtx temp, insn, set;
-
if (REG_P (x))
return x;
+ return force_recent (mode, x, force_reg_1);
+}
+
+/* Load X into a register if it is not already one.
+ Use mode MODE for the register.
+ Internal implementation of force_reg. */
+
+static rtx
+force_reg_1 (enum machine_mode mode, rtx x)
+{
+ rtx temp, insn, set;
+
if (general_operand (x, mode))
{
temp = gen_reg_rtx (mode);
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in.orig 2011-05-26 05:03:08.496988996 -0300
+++ gcc/Makefile.in 2011-05-26 05:03:31.904035421 -0300
@@ -2940,7 +2940,8 @@ except.o : except.c $(CONFIG_H) $(SYSTEM
expr.o : expr.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \
$(TREE_H) $(FLAGS_H) $(FUNCTION_H) $(REGS_H) $(EXPR_H) $(OPTABS_H) \
$(LIBFUNCS_H) $(INSN_ATTR_H) insn-config.h $(RECOG_H) output.h \
- typeclass.h hard-reg-set.h toplev.h $(DIAGNOSTIC_CORE_H) hard-reg-set.h $(EXCEPT_H) \
+ typeclass.h hard-reg-set.h toplev.h $(DIAGNOSTIC_CORE_H) \
+ gimple-pretty-print.h hard-reg-set.h $(EXCEPT_H) \
reload.h langhooks.h intl.h $(TM_P_H) $(TARGET_H) \
tree-iterator.h gt-expr.h $(MACHMODE_H) $(TIMEVAR_H) $(TREE_FLOW_H) \
$(TREE_PASS_H) $(DF_H) $(DIAGNOSTIC_H) vecprim.h $(SSAEXPAND_H)
[-- Attachment #3: Type: text/plain, Size: 258 bytes --]
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2011-05-30 13:25 [PR48866] three alternative fixes Alexandre Oliva
2011-05-30 13:34 ` Alexandre Oliva
2011-05-30 14:30 ` Alexandre Oliva
@ 2011-05-30 14:55 ` Alexandre Oliva
2011-05-30 15:35 ` Michael Matz
2011-06-03 1:34 ` Alexandre Oliva
2 siblings, 2 replies; 12+ messages in thread
From: Alexandre Oliva @ 2011-05-30 14:55 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]
On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
> 3. expand dominators before dominated blocks, so that DEFs of
> replaceable SSA names are expanded before their uses. Expand them when
> they're encountered, but not requiring a REG as a result. Save the RTL
> expression that results from the expansion for use in debug insns and at
> the non-debug use.
This patch addresses some of the problems in 2, avoiding expanding code
out of order within a block, and (hopefully) ensuring that, expanding
dominators before dominatedblocks, DEFs are expanded before USEs. There
is a theoretical possibility that a USE may be expanded before a DEF,
depending on internal details of out-of-ssa, but should this ever
happen, we'll get a failed assertion, and then disabling TER will work
around the problem.
Since no special handling of force_reg is required and we don't reorder
code, we don't need placeholders, and can record only the value
expansion of replaceable DEFs for their uses, debug or not.
Nevertheless, we expand each BB into a separate sequence, and then
re-emit the blocks into a single sequence in the original order. This
is a bit risky, as the logic of block expansion is modified, more so
because the blocks created during expansion don't get separate sequences
and require special handling, and other (unreachable?) blocks need to be
expanded in a separate loop.
This is the sort of code we get with this patch:
;; x_ = ****y_; (actually a sequence of replaceable gimple assignments)
(set (reg) (mem (reg y_)))
(set (reg) (mem (reg...)))
(set (reg) (mem (reg...)))
;; debug x => x_
(debug_insn (var_location x (mem (reg))))
[...]
;; ? = x_ + 1;
(set (reg x_) (mem (reg...)))
(set (reg ?) (plus (reg x_) (const_int 1)))
Note that the debug_insn binds to a computed expression, although that
same expression is later stored in a register. This is slightly
undesirable from a debug information POV, but it's probably something we
can live with.
No regressions in a regstrap on x86_64-linux-gnu and i686-linux-gnu.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: expand-replaceable-in-order-pr48866-7.patch --]
[-- Type: text/x-diff, Size: 7769 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/48866
* cfgexpand.c (def_expansions): New.
(def_expansion_recent_tree, def_expansion_recent_rtx): New.
(def_expansions_init, def_expansions_fini): New.
(def_has_expansion_ptr, def_get_expansion_ptr): New.
(expand_debug_expr): Use recorded expansion if available.
(expand_gimple_basic_block): Prepare to record expansion of
replaceable defs. Change return type to void.
(gimple_expand_cfg): Initialize and finalize expansions cache.
Expand dominator blocks before dominated.
* expr.c (expand_expr_real_1): Use recorded expansion of
replaceable defs.
* expr.h (def_has_expansion_ptr): Declare.
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig 2011-05-28 07:11:12.132275336 -0300
+++ gcc/cfgexpand.c 2011-05-28 07:19:55.128377121 -0300
@@ -2337,6 +2337,55 @@ convert_debug_memory_address (enum machi
return x;
}
+/* Map replaceable SSA_NAMEs to their RTL expansions. */
+static struct pointer_map_t *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 = pointer_map_create ();
+}
+
+/* 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)
+{
+ gcc_checking_assert (def_expansions);
+ pointer_map_destroy (def_expansions);
+ def_expansions = NULL;
+}
+
+/* Return NULL if no expansion is registered for EXP, or a pointer to
+ the rtx expanded from it otherwise. EXP must be a replaceable
+ SSA_NAME. */
+
+void *const *
+def_has_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 pointer_map_contains (def_expansions, exp);
+}
+
+/* Return a pointer to the rtx expanded from EXP. EXP must be a
+ replaceable SSA_NAME. */
+
+static void **
+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 pointer_map_insert (def_expansions, exp);
+}
+
/* Return an RTX equivalent to the value of the tree expression
EXP. */
@@ -3131,7 +3180,16 @@ 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));
+ void *const *xp = def_has_expansion_ptr (exp);
+
+ if (xp)
+ op0 = (rtx) *xp;
+ else
+ op0 = NULL;
+
+ if (!op0)
+ op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g));
+
if (!op0)
return NULL;
}
@@ -3346,7 +3404,7 @@ expand_debug_locations (void)
/* Expand basic block BB from GIMPLE trees to RTL. */
-static basic_block
+static void
expand_gimple_basic_block (basic_block bb)
{
gimple_stmt_iterator gsi;
@@ -3539,7 +3597,7 @@ expand_gimple_basic_block (basic_block b
{
new_bb = expand_gimple_cond (bb, stmt);
if (new_bb)
- return new_bb;
+ return;
}
else if (gimple_debug_bind_p (stmt))
{
@@ -3613,25 +3671,43 @@ expand_gimple_basic_block (basic_block b
if (can_fallthru)
bb = new_bb;
else
- return new_bb;
+ return;
}
}
else
{
+ void **xp = NULL;
def_operand_p def_p;
def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
- if (def_p != NULL)
+ /* Ignore this stmt if it is in the list of
+ replaceable expressions. */
+ 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))))
- continue;
+ tree def = DEF_FROM_PTR (def_p);
+ gimple g = get_gimple_for_ssa_name (def);
+ rtx retval;
+
+ last = get_last_insn ();
+
+ retval = expand_expr (gimple_assign_rhs_to_tree (g),
+ NULL_RTX, VOIDmode, EXPAND_SUM);
+
+ xp = def_get_expansion_ptr (def);
+ gcc_checking_assert (!*xp);
+ *xp = retval;
}
- last = expand_gimple_stmt (stmt);
+ else
+ last = expand_gimple_stmt (stmt);
maybe_dump_rtl_for_gimple_stmt (stmt, last);
+ if (xp && dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "\n=> ");
+ print_rtl (dump_file, (rtx) *xp);
+ }
}
}
}
@@ -3680,10 +3756,9 @@ expand_gimple_basic_block (basic_block b
update_bb_for_insn (bb);
- return bb;
+ return;
}
-
/* Create a basic block for initialization code. */
static basic_block
@@ -3969,6 +4044,9 @@ gimple_expand_cfg (void)
edge e;
rtx var_seq;
unsigned i;
+ VEC (basic_block, heap) *h;
+ int nblocks;
+
timevar_push (TV_OUT_OF_SSA);
rewrite_out_of_ssa (&SA);
@@ -4112,11 +4190,41 @@ gimple_expand_cfg (void)
e->flags &= ~EDGE_EXECUTABLE;
lab_rtx_for_bb = pointer_map_create ();
+ def_expansions_init ();
+
+ /* Expand dominators before dominated blocks, so that replaceable
+ DEFs are always expanded before their uses. */
+ nblocks = last_basic_block;
+ blocks = sbitmap_alloc (nblocks);
+ sbitmap_zero (blocks);
+
+ h = get_all_dominated_blocks (CDI_DOMINATORS, init_block->next_bb);
+
+ FOR_EACH_VEC_ELT (basic_block, h, i, bb)
+ {
+ gcc_checking_assert (bb->index < nblocks);
+ SET_BIT (blocks, bb->index);
+
+ start_sequence ();
+ emit_note (NOTE_INSN_DELETED);
+ expand_gimple_basic_block (bb);
+ end_sequence ();
+ }
+
+ VEC_free (basic_block, heap, h);
+
FOR_BB_BETWEEN (bb, init_block->next_bb, EXIT_BLOCK_PTR, next_bb)
- bb = expand_gimple_basic_block (bb);
+ if (bb->index >= nblocks)
+ continue;
+ else if (TEST_BIT (blocks, bb->index))
+ emit_insn (BB_HEAD (bb));
+ else
+ expand_gimple_basic_block (bb);
+ sbitmap_free (blocks);
if (MAY_HAVE_DEBUG_INSNS)
expand_debug_locations ();
+ def_expansions_fini ();
execute_free_datastructures ();
timevar_push (TV_OUT_OF_SSA);
Index: gcc/expr.c
===================================================================
--- gcc/expr.c.orig 2011-05-28 07:11:12.139275310 -0300
+++ gcc/expr.c 2011-05-28 07:11:14.895265175 -0300
@@ -8424,10 +8424,21 @@ expand_expr_real_1 (tree exp, rtx target
&& !SSA_NAME_IS_DEFAULT_DEF (exp)
&& (optimize || DECL_IGNORED_P (SSA_NAME_VAR (exp)))
&& stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp)))
- g = SSA_NAME_DEF_STMT (exp);
+ {
+ g = SSA_NAME_DEF_STMT (exp);
+ if (g)
+ return expand_expr_real (gimple_assign_rhs_to_tree (g),
+ target, tmode, modifier, NULL);
+ }
if (g)
- return expand_expr_real (gimple_assign_rhs_to_tree (g), target, tmode,
- modifier, NULL);
+ {
+ void *const *xp = def_has_expansion_ptr (exp);
+ rtx retval = (rtx) *xp;
+
+ gcc_assert (retval);
+
+ return retval;
+ }
ssa_name = exp;
decl_rtl = get_rtx_for_ssa_name (ssa_name);
Index: gcc/expr.h
===================================================================
--- gcc/expr.h.orig 2011-05-28 07:11:12.147275281 -0300
+++ gcc/expr.h 2011-05-28 07:16:13.428175792 -0300
@@ -693,4 +693,7 @@ extern tree build_libfunc_function (cons
/* Get the personality libfunc for a function decl. */
rtx get_personality_function (tree);
+/* In cfgexpand.c. */
+void *const *def_has_expansion_ptr (tree);
+
#endif /* GCC_EXPR_H */
[-- Attachment #3: Type: text/plain, Size: 258 bytes --]
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2011-05-30 14:55 ` Alexandre Oliva
@ 2011-05-30 15:35 ` Michael Matz
2011-06-01 22:31 ` Alexandre Oliva
2011-06-03 1:34 ` Alexandre Oliva
1 sibling, 1 reply; 12+ messages in thread
From: Michael Matz @ 2011-05-30 15:35 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: gcc-patches
Hi,
On Mon, 30 May 2011, Alexandre Oliva wrote:
> > 3. expand dominators before dominated blocks, so that DEFs of
> > replaceable SSA names are expanded before their uses. Expand them
> > when they're encountered, but not requiring a REG as a result.
> > Save the RTL expression that results from the expansion for use in
> > debug insns and at the non-debug use.
>
> This patch addresses some of the problems in 2, avoiding expanding code
> out of order within a block, and (hopefully) ensuring that, expanding
> dominators before dominatedblocks, DEFs are expanded before USEs.
That isn't necessary. Replaceable SSA_NAMEs are defined in the same BB as
their use, and hence they're expanded strictly before their use already
right now.
> There is a theoretical possibility that a USE may be expanded before a
> DEF, depending on internal details of out-of-ssa,
This can only happen with non-replaceable SSA_NAMEs and I thought you wish
to deal only with replaceable.
That said, I dislike approach 2 for the same worries you noted. And with
the above I don't see why your approach (3) isn't workable without changes
to the BB order, which should then be a really small patch. As SSA_NAMEs
are (fairly) dense it might be better to simply use an array instead of a
pointer_map for the SSA_NAME->rtx mapping.
Ciao,
Michael.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2011-05-30 15:35 ` Michael Matz
@ 2011-06-01 22:31 ` Alexandre Oliva
0 siblings, 0 replies; 12+ messages in thread
From: Alexandre Oliva @ 2011-06-01 22:31 UTC (permalink / raw)
To: Michael Matz; +Cc: gcc-patches
On May 30, 2011, Michael Matz <matz@suse.de> wrote:
> Hi,
> On Mon, 30 May 2011, Alexandre Oliva wrote:
>> > 3. expand dominators before dominated blocks, so that DEFs of
>> > replaceable SSA names are expanded before their uses. Expand them
>> > when they're encountered, but not requiring a REG as a result.
>> > Save the RTL expression that results from the expansion for use in
>> > debug insns and at the non-debug use.
>>
>> This patch addresses some of the problems in 2, avoiding expanding code
>> out of order within a block, and (hopefully) ensuring that, expanding
>> dominators before dominatedblocks, DEFs are expanded before USEs.
> That isn't necessary. Replaceable SSA_NAMEs are defined in the same BB as
> their use, and hence they're expanded strictly before their use already
> right now.
Hmm... You're obviously right. I must have misinterpreted some other
failure, then. Thanks, this enables some simplification to two of the
patches.
>> There is a theoretical possibility that a USE may be expanded before a
>> DEF, depending on internal details of out-of-ssa,
> This can only happen with non-replaceable SSA_NAMEs and I thought you wish
> to deal only with replaceable.
Yup.
> That said, I dislike approach 2 for the same worries you noted. And with
> the above I don't see why your approach (3) isn't workable without changes
> to the BB order, which should then be a really small patch.
Yup. I'll give that a try.
> As SSA_NAMEs are (fairly) dense it might be better to simply use an
> array instead of a pointer_map for the SSA_NAME->rtx mapping.
Likewise.
Thanks!
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2011-05-30 13:34 ` Alexandre Oliva
@ 2011-06-03 1:21 ` Alexandre Oliva
2012-04-09 6:08 ` Alexandre Oliva
0 siblings, 1 reply; 12+ messages in thread
From: Alexandre Oliva @ 2011-06-03 1:21 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]
On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
>> 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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: expand-replaceable-for-debug-only-pr48866.patch --]
[-- Type: text/x-diff, Size: 14741 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/48866
* cfgexpand.c (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.
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig 2011-06-01 19:45:02.520428653 -0300
+++ gcc/cfgexpand.c 2011-06-01 20:20:02.014975168 -0300
@@ -2337,6 +2337,70 @@ convert_debug_memory_address (enum machi
return x;
}
+/* 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. */
@@ -3131,7 +3195,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;
}
@@ -3293,6 +3380,45 @@ expand_debug_expr (tree exp)
}
}
+/* 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 ();
+ 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. */
@@ -3301,7 +3427,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
@@ -3310,38 +3435,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
- {
- 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. */
@@ -3433,104 +3574,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
@@ -3543,64 +3586,15 @@ 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 (gimple_debug_bind_has_value_p (stmt))
- value = gimple_debug_bind_get_value (stmt);
- else
- value = NULL_TREE;
-
- last = get_last_insn ();
+ tree var = gimple_debug_bind_get_var (stmt);
+ tree value;
- 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);
-
- 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;
- }
-
- /* 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
{
@@ -3621,14 +3615,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);
@@ -3970,6 +3979,7 @@ gimple_expand_cfg (void)
rtx var_seq;
unsigned i;
+
timevar_push (TV_OUT_OF_SSA);
rewrite_out_of_ssa (&SA);
timevar_pop (TV_OUT_OF_SSA);
@@ -4112,11 +4122,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);
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2011-05-30 14:30 ` Alexandre Oliva
@ 2011-06-03 1:28 ` Alexandre Oliva
2011-06-03 1:37 ` Alexandre Oliva
1 sibling, 0 replies; 12+ messages in thread
From: Alexandre Oliva @ 2011-06-03 1:28 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]
On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
>> 2. emit placeholders for replaceable DEFs and, when the DEFs are
>> expanded at their point of use, emit the expansion next to the
>> placeholder, rather than at the current stream. The result of the
>> expansion is saved and used in debug insns that reference the
>> replaceable DEF. If the result is forced into a REG shortly thereafter,
>> the code resulting from this is also emitted next to the placeholder,
>> and the saved expansion is updated. If the USE is expanded before the
>> DEF, the insn stream resulting from the expansion is saved and emitted
>> at the point of the DEF.
> IMHO this is the riskiest of the 3 patches, for shuffling expansions
> around isn't exactly something I'm comfortable with. There's a very
> real risk that moving the expansion of sub-expressions to their
> definition points may end up moving uses before definitions.
Upthread, I posted the wrong patch: instead of the one that tolerated
expanding DEFs before or after USEs, I posted a simplifying experiment
that seemed to fail, but it looks like I misinterpreted the results.
This revised and retested patch also records expansions in an array
rather than a pointer_map, and it avoids re-expanding DEFs when a USE is
expanded for the second time. Although replaceable DEFs can only have
one USE, when the single USE appears in a call stmt, it can be expanded
twice. I'm not sure whether it would be better to expand it twice and
let RTL optimizations drop any redundancies, or reuse the result of the
first expansion, like this patch does.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: expand-replaceable-back-in-place-pr48866.patch --]
[-- Type: text/x-diff, Size: 13237 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/48866
* cfgexpand.c (def_expansions): New.
(def_expansion_recent_tree, def_expansion_recent_rtx): New.
(def_expansions_init): New.
(def_expansions_remove_placeholder, def_expansions_fini): New.
(def_get_expansion_ptr): New.
(def_expansion_recent, def_expansion_record_recent): New.
(def_expansion_add_insns): New.
(expand_debug_expr): Use recorded expansion if available.
(expand_gimple_basic_block): Prepare to record expansion of
replaceable defs. Reset recent expansions at the end of the
block.
(gimple_expand_cfg): Initialize and finalize expansions cache.
* expr.c: Include gimple-pretty-print.h.
(store_expr): Forget recent expansions upon nontemporal moves.
(expand_expr_real_1): Reuse or record expansion of replaceable
defs.
* expr.h (def_get_expansion_ptr, def_expansion_recent): Declare.
(def_expansion_record_recent, def_expansion_add_insns): Declare.
* explow.c (force_recent): New.
(force_reg): Use it. Split into...
(force_reg_1): ... this.
* Makefile.in (expr.o): Depend on gimple-pretty-print.h.
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig 2011-06-01 20:21:01.692928212 -0300
+++ gcc/cfgexpand.c 2011-06-01 20:32:45.289277086 -0300
@@ -2337,6 +2337,144 @@ convert_debug_memory_address (enum machi
return x;
}
+/* Map replaceable SSA_NAMEs to NOTE_INSN_VAR_LOCATIONs that hold
+ their RTL expansions (once available) in their NOTE_VAR_LOCATIONs
+ (without a VAR_LOCATION rtx). The SSA_NAME DEF is expanded before
+ its single USE, so the NOTE is inserted in the insn stream, marking
+ the location where the non-replaceable portion of the expansion is
+ to be inserted. When the single USE is expanded, it will be
+ emitted before the NOTE. */
+static rtx *def_expansions;
+
+/* The latest expanded SSA name, and its corresponding RTL expansion.
+ These are used to enable the insertion of the insn that stores the
+ expansion in a register at the end of the sequence expanded for the
+ SSA DEF. */
+static tree def_expansion_recent_tree;
+static rtx def_expansion_recent_rtx;
+
+/* 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 = XNEWVEC (rtx, num_ssa_names);
+
+ gcc_checking_assert (!def_expansion_recent_tree);
+ gcc_checking_assert (!def_expansion_recent_rtx);
+}
+
+/* Remove the NOTE that marks the insertion location of the expansion
+ of a replaceable SSA note. */
+
+static bool
+def_expansions_remove_placeholder (rtx note)
+{
+ if (!note)
+ return true;
+
+ gcc_checking_assert (NOTE_P (note));
+ remove_insn (note);
+
+ 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;
+ def_expansion_recent_tree = NULL;
+ def_expansion_recent_rtx = NULL;
+}
+
+/* Return a pointer to the NOTE marking the insertion point for the
+ expansion of EXP. EXP must be a replaceable SSA_NAME. */
+
+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)];
+}
+
+/* Return an SSA name, if any, that was recently expanded to the value
+ X. */
+
+tree
+def_expansion_recent (rtx x)
+{
+ if (!def_expansion_recent_rtx)
+ return NULL;
+
+ if (x == def_expansion_recent_rtx
+ || rtx_equal_p (x, def_expansion_recent_rtx))
+ return def_expansion_recent_tree;
+
+ return NULL;
+}
+
+/* Record that DEF was recently expanded to the value X. */
+
+void
+def_expansion_record_recent (tree def, rtx x)
+{
+ def_expansion_recent_tree = def;
+ def_expansion_recent_rtx = x;
+}
+
+/* Forget recent expansions. */
+
+void
+def_expansion_reset_recent (void)
+{
+ def_expansion_record_recent (NULL, NULL);
+}
+
+/* Add INSNS to the insn seq generated for DEF, and update the
+ RTL value of DEF to VAL. */
+
+void
+def_expansion_add_insns (tree def, rtx insns, rtx val)
+{
+ rtx note = *def_get_expansion_ptr (def);
+
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "\n;; (cont) ");
+ print_gimple_stmt (dump_file, SSA_NAME_DEF_STMT (def), 0,
+ TDF_SLIM | (dump_flags & TDF_LINENO));
+ fprintf (dump_file, "\n");
+
+ print_rtl (dump_file, insns);
+
+ fprintf (dump_file, "\n=> ");
+ print_rtl (dump_file, val);
+ }
+
+ gcc_checking_assert (note);
+ emit_insn_before (insns, note);
+ gcc_checking_assert (NOTE_VAR_LOCATION (note));
+ NOTE_VAR_LOCATION (note) = val;
+
+ def_expansion_recent_tree = NULL;
+ def_expansion_recent_rtx = NULL;
+}
+
/* Return an RTX equivalent to the value of the tree expression
EXP. */
@@ -3131,7 +3269,20 @@ 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 *xp = def_get_expansion_ptr (exp);
+
+ if (xp)
+ {
+ rtx note = *xp;
+ gcc_checking_assert (NOTE_P (note));
+ op0 = NOTE_VAR_LOCATION (note);
+ }
+ else
+ op0 = NULL;
+
+ if (!op0)
+ op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g));
+
if (!op0)
return NULL;
}
@@ -3621,22 +3772,34 @@ 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)
+ /* Ignore this stmt if it is in the list of
+ replaceable expressions. */
+ 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))))
- continue;
+ tree def = DEF_FROM_PTR (def_p);
+ rtx *xp = def_get_expansion_ptr (def);
+ rtx note;
+
+ last = get_last_insn ();
+
+ gcc_checking_assert (!*xp);
+
+ note = emit_note (NOTE_INSN_VAR_LOCATION);
+ NOTE_VAR_LOCATION (note) = NULL;
+ *xp = note;
}
- last = expand_gimple_stmt (stmt);
+ else
+ last = expand_gimple_stmt (stmt);
maybe_dump_rtl_for_gimple_stmt (stmt, last);
}
}
}
currently_expanding_gimple_stmt = NULL;
+ def_expansion_reset_recent ();
/* Expand implicit goto and convert goto_locus. */
FOR_EACH_EDGE (e, ei, bb->succs)
@@ -4112,11 +4275,14 @@ gimple_expand_cfg (void)
e->flags &= ~EDGE_EXECUTABLE;
lab_rtx_for_bb = pointer_map_create ();
+ 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 ();
+ def_expansions_fini ();
execute_free_datastructures ();
timevar_push (TV_OUT_OF_SSA);
Index: gcc/expr.c
===================================================================
--- gcc/expr.c.orig 2011-06-01 20:21:00.837928892 -0300
+++ gcc/expr.c 2011-06-01 20:37:28.144975193 -0300
@@ -48,6 +48,7 @@ along with GCC; see the file COPYING3.
#include "tree-iterator.h"
#include "tree-pass.h"
#include "tree-flow.h"
+#include "gimple-pretty-print.h"
#include "target.h"
#include "timevar.h"
#include "df.h"
@@ -4675,6 +4676,9 @@ store_expr (tree exp, rtx target, int ca
(call_param_p
? EXPAND_STACK_PARM : EXPAND_NORMAL),
&alt_rtl);
+ /* Don't risk moving loads before stores. */
+ if (!nontemporal)
+ def_expansion_reset_recent ();
}
/* If TEMP is a VOIDmode constant and the mode of the type of EXP is not
@@ -8424,10 +8428,59 @@ expand_expr_real_1 (tree exp, rtx target
&& !SSA_NAME_IS_DEFAULT_DEF (exp)
&& (optimize || DECL_IGNORED_P (SSA_NAME_VAR (exp)))
&& stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp)))
- g = SSA_NAME_DEF_STMT (exp);
+ {
+ g = SSA_NAME_DEF_STMT (exp);
+ if (g)
+ return expand_expr_real (gimple_assign_rhs_to_tree (g),
+ target, tmode, modifier, NULL);
+ }
if (g)
- return expand_expr_real (gimple_assign_rhs_to_tree (g), target, tmode,
- modifier, NULL);
+ {
+ rtx retval;
+ rtx insns;
+ rtx note = *def_get_expansion_ptr (exp);
+
+ gcc_assert (NOTE_P (note));
+
+ start_sequence ();
+
+ retval = expand_expr_real (gimple_assign_rhs_to_tree (g),
+ target, tmode, modifier, NULL);
+
+ insns = get_insns ();
+
+ if (retval == target)
+ {
+ end_sequence ();
+ emit_insn (insns);
+ return retval;
+ }
+
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "\n;; ");
+ print_gimple_stmt (dump_file, g, 0,
+ TDF_SLIM | (dump_flags & TDF_LINENO));
+ fprintf (dump_file, "\n");
+
+ print_rtl (dump_file, insns);
+
+ fprintf (dump_file, "\n=> ");
+ print_rtl (dump_file, retval);
+ }
+
+ end_sequence ();
+
+ /* Make sure this is in the instruction stream. */
+ gcc_checking_assert (PREV_INSN (note));
+ emit_insn_before (insns, note);
+ gcc_checking_assert (!NOTE_VAR_LOCATION (note));
+
+ NOTE_VAR_LOCATION (note) = retval;
+ def_expansion_record_recent (exp, retval);
+
+ return retval;
+ }
ssa_name = exp;
decl_rtl = get_rtx_for_ssa_name (ssa_name);
Index: gcc/expr.h
===================================================================
--- gcc/expr.h.orig 2011-06-01 20:21:01.012928748 -0300
+++ gcc/expr.h 2011-06-01 20:33:47.056211898 -0300
@@ -693,4 +693,11 @@ extern tree build_libfunc_function (cons
/* Get the personality libfunc for a function decl. */
rtx get_personality_function (tree);
+/* In cfgexpand.c. */
+rtx *def_get_expansion_ptr (tree);
+tree def_expansion_recent (rtx);
+void def_expansion_record_recent (tree, rtx);
+void def_expansion_reset_recent (void);
+void def_expansion_add_insns (tree, rtx, rtx);
+
#endif /* GCC_EXPR_H */
Index: gcc/explow.c
===================================================================
--- gcc/explow.c.orig 2011-05-28 07:11:12.153275260 -0300
+++ gcc/explow.c 2011-06-01 20:25:37.798693545 -0300
@@ -638,22 +638,62 @@ copy_to_mode_reg (enum machine_mode mode
return temp;
}
+/* If X is the value of a recently-expanded SSA def, emit the insns
+ generated by FN (MODE, X) at the end of the expansion of that def,
+ otherwise emit them in the current insn seq. */
+
+static inline rtx
+force_recent (enum machine_mode mode, rtx x,
+ rtx (*fn) (enum machine_mode, rtx))
+{
+ tree def = def_expansion_recent (x);
+ rtx retval;
+ rtx insns;
+
+ if (!def)
+ return fn (mode, x);
+
+ start_sequence ();
+ retval = fn (mode, x);
+ insns = get_insns ();
+ end_sequence ();
+
+ def_expansion_add_insns (def, insns, retval);
+
+ return retval;
+}
+
+static rtx force_reg_1 (enum machine_mode, rtx);
+
/* Load X into a register if it is not already one.
Use mode MODE for the register.
X should be valid for mode MODE, but it may be a constant which
is valid for all integer modes; that's why caller must specify MODE.
+ If X is the value of a recent SSA def expansion, emit the insns at
+ the end of that expansion, rather than into the current seq.
+
The caller must not alter the value in the register we return,
since we mark it as a "constant" register. */
rtx
force_reg (enum machine_mode mode, rtx x)
{
- rtx temp, insn, set;
-
if (REG_P (x))
return x;
+ return force_recent (mode, x, force_reg_1);
+}
+
+/* Load X into a register if it is not already one.
+ Use mode MODE for the register.
+ Internal implementation of force_reg. */
+
+static rtx
+force_reg_1 (enum machine_mode mode, rtx x)
+{
+ rtx temp, insn, set;
+
if (general_operand (x, mode))
{
temp = gen_reg_rtx (mode);
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in.orig 2011-05-30 03:53:29.047724372 -0300
+++ gcc/Makefile.in 2011-06-01 20:25:37.855693494 -0300
@@ -2942,7 +2942,8 @@ except.o : except.c $(CONFIG_H) $(SYSTEM
expr.o : expr.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \
$(TREE_H) $(FLAGS_H) $(FUNCTION_H) $(REGS_H) $(EXPR_H) $(OPTABS_H) \
$(LIBFUNCS_H) $(INSN_ATTR_H) insn-config.h $(RECOG_H) output.h \
- typeclass.h hard-reg-set.h toplev.h $(DIAGNOSTIC_CORE_H) hard-reg-set.h $(EXCEPT_H) \
+ typeclass.h hard-reg-set.h toplev.h $(DIAGNOSTIC_CORE_H) \
+ gimple-pretty-print.h hard-reg-set.h $(EXCEPT_H) \
reload.h langhooks.h intl.h $(TM_P_H) $(TARGET_H) \
tree-iterator.h gt-expr.h $(MACHMODE_H) $(TIMEVAR_H) $(TREE_FLOW_H) \
$(TREE_PASS_H) $(DF_H) $(DIAGNOSTIC_H) vecprim.h $(SSAEXPAND_H)
[-- Attachment #3: Type: text/plain, Size: 258 bytes --]
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2011-05-30 14:55 ` Alexandre Oliva
2011-05-30 15:35 ` Michael Matz
@ 2011-06-03 1:34 ` Alexandre Oliva
1 sibling, 0 replies; 12+ messages in thread
From: Alexandre Oliva @ 2011-06-03 1:34 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1513 bytes --]
On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
>> 3. expand dominators before dominated blocks, so that DEFs of
>> replaceable SSA names are expanded before their uses. Expand them when
>> they're encountered, but not requiring a REG as a result. Save the RTL
>> expression that results from the expansion for use in debug insns and at
>> the non-debug use.
> This patch addresses some of the problems in 2, avoiding expanding code
> out of order within a block, and (hopefully) ensuring that, expanding
> dominators before dominatedblocks, DEFs are expanded before USEs. There
> is a theoretical possibility that a USE may be expanded before a DEF,
> depending on internal details of out-of-ssa, but should this ever
> happen, we'll get a failed assertion, and then disabling TER will work
> around the problem.
I also posted the wrong patch upthread for this variant. The one I
posted didn't work at all, because it contained a last-minute
optimization that changed the expansion of replaceable stmts from
EXPAND_NORMAL to EXPAND_SUM. IIRC the former always yielded a pseudo,
whereas the former enabled replacements, but it also exposed the need
for better handling of non-general_operands when the use expects one.
This revised and retested version also drops the reordering of the
expansion of basic blocks, that Matz pointed out was unnecessary, and
switches to an array rather than a pointer_map to record the expansions.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: expand-replaceable-in-place-pr48866.patch --]
[-- Type: text/x-diff, Size: 6004 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/48866
* cfgexpand.c (def_expansions): New.
(def_expansion_recent_tree, def_expansion_recent_rtx): New.
(def_expansions_init, def_expansions_fini): New.
(def_has_expansion_ptr, def_get_expansion_ptr): New.
(expand_debug_expr): Use recorded expansion if available.
(expand_gimple_basic_block): Prepare to record expansion of
replaceable defs. Change return type to void.
(gimple_expand_cfg): Initialize and finalize expansions cache.
Expand dominator blocks before dominated.
* expr.c (expand_expr_real_1): Use recorded expansion of
replaceable defs.
* expr.h (def_has_expansion_ptr): Declare.
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig 2011-06-01 20:39:58.244953408 -0300
+++ gcc/cfgexpand.c 2011-06-01 21:44:38.005879125 -0300
@@ -2337,6 +2337,42 @@ convert_debug_memory_address (enum machi
return x;
}
+/* Map replaceable SSA_NAMEs 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);
+}
+
+/* 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)
+{
+ gcc_checking_assert (def_expansions);
+ XDELETEVEC (def_expansions);
+ def_expansions = NULL;
+}
+
+/* Return a pointer to the rtx expanded from EXP. EXP must be a
+ replaceable SSA_NAME. */
+
+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)];
+}
+
/* Return an RTX equivalent to the value of the tree expression
EXP. */
@@ -3131,7 +3167,16 @@ 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 *xp = def_get_expansion_ptr (exp);
+
+ if (xp)
+ op0 = copy_rtx (*xp);
+ else
+ op0 = NULL;
+
+ if (!op0)
+ op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g));
+
if (!op0)
return NULL;
}
@@ -3618,20 +3663,38 @@ expand_gimple_basic_block (basic_block b
}
else
{
+ rtx *xp = NULL;
def_operand_p def_p;
def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
- if (def_p != NULL)
+ /* Ignore this stmt if it is in the list of
+ replaceable expressions. */
+ 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))))
- continue;
+ tree def = DEF_FROM_PTR (def_p);
+ gimple g = get_gimple_for_ssa_name (def);
+ rtx retval;
+
+ last = get_last_insn ();
+
+ retval = expand_expr (gimple_assign_rhs_to_tree (g),
+ NULL_RTX, VOIDmode, EXPAND_SUM);
+
+ xp = def_get_expansion_ptr (def);
+ gcc_checking_assert (!*xp);
+ *xp = retval;
}
- last = expand_gimple_stmt (stmt);
+ else
+ last = expand_gimple_stmt (stmt);
maybe_dump_rtl_for_gimple_stmt (stmt, last);
+ if (xp && dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "\n=> ");
+ print_rtl (dump_file, *xp);
+ }
}
}
}
@@ -4112,11 +4175,14 @@ gimple_expand_cfg (void)
e->flags &= ~EDGE_EXECUTABLE;
lab_rtx_for_bb = pointer_map_create ();
+ 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 ();
+ def_expansions_fini ();
execute_free_datastructures ();
timevar_push (TV_OUT_OF_SSA);
Index: gcc/expr.c
===================================================================
--- gcc/expr.c.orig 2011-06-01 20:39:58.604953351 -0300
+++ gcc/expr.c 2011-06-01 22:44:52.659524628 -0300
@@ -8424,10 +8424,40 @@ expand_expr_real_1 (tree exp, rtx target
&& !SSA_NAME_IS_DEFAULT_DEF (exp)
&& (optimize || DECL_IGNORED_P (SSA_NAME_VAR (exp)))
&& stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp)))
- g = SSA_NAME_DEF_STMT (exp);
+ {
+ g = SSA_NAME_DEF_STMT (exp);
+ if (g)
+ return expand_expr_real (gimple_assign_rhs_to_tree (g),
+ target, tmode, modifier, NULL);
+ }
if (g)
- return expand_expr_real (gimple_assign_rhs_to_tree (g), target, tmode,
- modifier, NULL);
+ {
+ rtx retval = *def_get_expansion_ptr (exp);
+
+ gcc_assert (retval);
+
+ switch (modifier)
+ {
+ case EXPAND_SUM:
+ return retval;
+
+ case EXPAND_STACK_PARM:
+ case EXPAND_NORMAL:
+ if (!target || !REG_P (target) || GET_MODE (target) != mode
+ || !general_operand (retval, mode))
+ return force_reg (mode, retval);
+
+ emit_move_insn (target, retval);
+ return target;
+
+ case EXPAND_MEMORY:
+ case EXPAND_CONST_ADDRESS:
+ case EXPAND_INITIALIZER:
+ case EXPAND_WRITE:
+ default:
+ gcc_unreachable ();
+ }
+ }
ssa_name = exp;
decl_rtl = get_rtx_for_ssa_name (ssa_name);
Index: gcc/expr.h
===================================================================
--- gcc/expr.h.orig 2011-06-01 20:39:58.742953327 -0300
+++ gcc/expr.h 2011-06-01 20:47:22.640844048 -0300
@@ -693,4 +693,7 @@ extern tree build_libfunc_function (cons
/* Get the personality libfunc for a function decl. */
rtx get_personality_function (tree);
+/* In cfgexpand.c. */
+rtx *def_get_expansion_ptr (tree);
+
#endif /* GCC_EXPR_H */
[-- Attachment #3: Type: text/plain, Size: 258 bytes --]
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2011-05-30 14:30 ` Alexandre Oliva
2011-06-03 1:28 ` Alexandre Oliva
@ 2011-06-03 1:37 ` Alexandre Oliva
1 sibling, 0 replies; 12+ messages in thread
From: Alexandre Oliva @ 2011-06-03 1:37 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]
Ugh, failed to refresh the patch file, resending with the correct one.
On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
>> 2. emit placeholders for replaceable DEFs and, when the DEFs are
>> expanded at their point of use, emit the expansion next to the
>> placeholder, rather than at the current stream. The result of the
>> expansion is saved and used in debug insns that reference the
>> replaceable DEF. If the result is forced into a REG shortly thereafter,
>> the code resulting from this is also emitted next to the placeholder,
>> and the saved expansion is updated. If the USE is expanded before the
>> DEF, the insn stream resulting from the expansion is saved and emitted
>> at the point of the DEF.
> IMHO this is the riskiest of the 3 patches, for shuffling expansions
> around isn't exactly something I'm comfortable with. There's a very
> real risk that moving the expansion of sub-expressions to their
> definition points may end up moving uses before definitions.
Upthread, I posted the wrong patch: instead of the one that tolerated
expanding DEFs before or after USEs, I posted a simplifying experiment
that seemed to fail, but it looks like I misinterpreted the results.
This revised and retested patch also records expansions in an array
rather than a pointer_map, and it avoids re-expanding DEFs when a USE is
expanded for the second time. Although replaceable DEFs can only have
one USE, when the single USE appears in a call stmt, it can be expanded
twice. I'm not sure whether it would be better to expand it twice and
let RTL optimizations drop any redundancies, or reuse the result of the
first expansion, like this patch does.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: expand-replaceable-back-in-place-pr48866.patch --]
[-- Type: text/x-diff, Size: 13410 bytes --]
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/48866
* cfgexpand.c (def_expansions): New.
(def_expansion_recent_tree, def_expansion_recent_rtx): New.
(def_expansions_init): New.
(def_expansions_remove_placeholder, def_expansions_fini): New.
(def_get_expansion_ptr): New.
(def_expansion_recent, def_expansion_record_recent): New.
(def_expansion_add_insns): New.
(expand_debug_expr): Use recorded expansion if available.
(expand_gimple_basic_block): Prepare to record expansion of
replaceable defs. Reset recent expansions at the end of the
block.
(gimple_expand_cfg): Initialize and finalize expansions cache.
* expr.c: Include gimple-pretty-print.h.
(store_expr): Forget recent expansions upon nontemporal moves.
(expand_expr_real_1): Reuse or record expansion of replaceable
defs.
* expr.h (def_get_expansion_ptr, def_expansion_recent): Declare.
(def_expansion_record_recent, def_expansion_add_insns): Declare.
* explow.c (force_recent): New.
(force_reg): Use it. Split into...
(force_reg_1): ... this.
* Makefile.in (expr.o): Depend on gimple-pretty-print.h.
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig 2011-06-02 16:43:03.596818720 -0300
+++ gcc/cfgexpand.c 2011-06-02 17:18:10.217974612 -0300
@@ -2337,6 +2337,144 @@ convert_debug_memory_address (enum machi
return x;
}
+/* Map replaceable SSA_NAMEs to NOTE_INSN_VAR_LOCATIONs that hold
+ their RTL expansions (once available) in their NOTE_VAR_LOCATIONs
+ (without a VAR_LOCATION rtx). The SSA_NAME DEF is expanded before
+ its single USE, so the NOTE is inserted in the insn stream, marking
+ the location where the non-replaceable portion of the expansion is
+ to be inserted. When the single USE is expanded, it will be
+ emitted before the NOTE. */
+static rtx *def_expansions;
+
+/* The latest expanded SSA name, and its corresponding RTL expansion.
+ These are used to enable the insertion of the insn that stores the
+ expansion in a register at the end of the sequence expanded for the
+ SSA DEF. */
+static tree def_expansion_recent_tree;
+static rtx def_expansion_recent_rtx;
+
+/* 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);
+
+ gcc_checking_assert (!def_expansion_recent_tree);
+ gcc_checking_assert (!def_expansion_recent_rtx);
+}
+
+/* Remove the NOTE that marks the insertion location of the expansion
+ of a replaceable SSA note. */
+
+static bool
+def_expansions_remove_placeholder (rtx note)
+{
+ if (!note)
+ return true;
+
+ gcc_checking_assert (NOTE_P (note));
+ remove_insn (note);
+
+ 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;
+ def_expansion_recent_tree = NULL;
+ def_expansion_recent_rtx = NULL;
+}
+
+/* Return a pointer to the NOTE marking the insertion point for the
+ expansion of EXP. EXP must be a replaceable SSA_NAME. */
+
+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)];
+}
+
+/* Return an SSA name, if any, that was recently expanded to the value
+ X. */
+
+tree
+def_expansion_recent (rtx x)
+{
+ if (!def_expansion_recent_rtx)
+ return NULL;
+
+ if (x == def_expansion_recent_rtx
+ || rtx_equal_p (x, def_expansion_recent_rtx))
+ return def_expansion_recent_tree;
+
+ return NULL;
+}
+
+/* Record that DEF was recently expanded to the value X. */
+
+void
+def_expansion_record_recent (tree def, rtx x)
+{
+ def_expansion_recent_tree = def;
+ def_expansion_recent_rtx = x;
+}
+
+/* Forget recent expansions. */
+
+void
+def_expansion_reset_recent (void)
+{
+ def_expansion_record_recent (NULL, NULL);
+}
+
+/* Add INSNS to the insn seq generated for DEF, and update the
+ RTL value of DEF to VAL. */
+
+void
+def_expansion_add_insns (tree def, rtx insns, rtx val)
+{
+ rtx note = *def_get_expansion_ptr (def);
+
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "\n;; (cont) ");
+ print_gimple_stmt (dump_file, SSA_NAME_DEF_STMT (def), 0,
+ TDF_SLIM | (dump_flags & TDF_LINENO));
+ fprintf (dump_file, "\n");
+
+ print_rtl (dump_file, insns);
+
+ fprintf (dump_file, "\n=> ");
+ print_rtl (dump_file, val);
+ }
+
+ gcc_checking_assert (note);
+ emit_insn_before (insns, note);
+ gcc_checking_assert (NOTE_VAR_LOCATION (note));
+ NOTE_VAR_LOCATION (note) = val;
+
+ def_expansion_recent_tree = NULL;
+ def_expansion_recent_rtx = NULL;
+}
+
/* Return an RTX equivalent to the value of the tree expression
EXP. */
@@ -3131,7 +3269,20 @@ 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 *xp = def_get_expansion_ptr (exp);
+
+ if (xp)
+ {
+ rtx note = *xp;
+ gcc_checking_assert (NOTE_P (note));
+ op0 = NOTE_VAR_LOCATION (note);
+ }
+ else
+ op0 = NULL;
+
+ if (!op0)
+ op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g));
+
if (!op0)
return NULL;
}
@@ -3621,22 +3772,34 @@ 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)
+ /* Ignore this stmt if it is in the list of
+ replaceable expressions. */
+ 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))))
- continue;
+ tree def = DEF_FROM_PTR (def_p);
+ rtx *xp = def_get_expansion_ptr (def);
+ rtx note;
+
+ last = get_last_insn ();
+
+ gcc_checking_assert (!*xp);
+
+ note = emit_note (NOTE_INSN_VAR_LOCATION);
+ NOTE_VAR_LOCATION (note) = NULL;
+ *xp = note;
}
- last = expand_gimple_stmt (stmt);
+ else
+ last = expand_gimple_stmt (stmt);
maybe_dump_rtl_for_gimple_stmt (stmt, last);
}
}
}
currently_expanding_gimple_stmt = NULL;
+ def_expansion_reset_recent ();
/* Expand implicit goto and convert goto_locus. */
FOR_EACH_EDGE (e, ei, bb->succs)
@@ -4112,11 +4275,14 @@ gimple_expand_cfg (void)
e->flags &= ~EDGE_EXECUTABLE;
lab_rtx_for_bb = pointer_map_create ();
+ 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 ();
+ def_expansions_fini ();
execute_free_datastructures ();
timevar_push (TV_OUT_OF_SSA);
Index: gcc/expr.c
===================================================================
--- gcc/expr.c.orig 2011-06-02 02:16:06.027421293 -0300
+++ gcc/expr.c 2011-06-02 18:15:47.941701093 -0300
@@ -48,6 +48,7 @@ along with GCC; see the file COPYING3.
#include "tree-iterator.h"
#include "tree-pass.h"
#include "tree-flow.h"
+#include "gimple-pretty-print.h"
#include "target.h"
#include "timevar.h"
#include "df.h"
@@ -4675,6 +4676,9 @@ store_expr (tree exp, rtx target, int ca
(call_param_p
? EXPAND_STACK_PARM : EXPAND_NORMAL),
&alt_rtl);
+ /* Don't risk moving loads before stores. */
+ if (!nontemporal)
+ def_expansion_reset_recent ();
}
/* If TEMP is a VOIDmode constant and the mode of the type of EXP is not
@@ -8424,10 +8428,63 @@ expand_expr_real_1 (tree exp, rtx target
&& !SSA_NAME_IS_DEFAULT_DEF (exp)
&& (optimize || DECL_IGNORED_P (SSA_NAME_VAR (exp)))
&& stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp)))
- g = SSA_NAME_DEF_STMT (exp);
+ {
+ g = SSA_NAME_DEF_STMT (exp);
+ if (g)
+ return expand_expr_real (gimple_assign_rhs_to_tree (g),
+ target, tmode, modifier, NULL);
+ }
if (g)
- return expand_expr_real (gimple_assign_rhs_to_tree (g), target, tmode,
- modifier, NULL);
+ {
+ rtx retval;
+ rtx insns;
+ rtx note = *def_get_expansion_ptr (exp);
+
+ gcc_assert (NOTE_P (note));
+ /* Call parameters may be expanded twice. Reuse the result
+ of the first expansion. */
+ if (NOTE_VAR_LOCATION (note))
+ return NOTE_VAR_LOCATION (note);
+
+ start_sequence ();
+
+ retval = expand_expr_real (gimple_assign_rhs_to_tree (g),
+ target, tmode, modifier, NULL);
+
+ insns = get_insns ();
+
+ if (retval == target)
+ {
+ end_sequence ();
+ emit_insn (insns);
+ return retval;
+ }
+
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "\n;; ");
+ print_gimple_stmt (dump_file, g, 0,
+ TDF_SLIM | (dump_flags & TDF_LINENO));
+ fprintf (dump_file, "\n");
+
+ print_rtl (dump_file, insns);
+
+ fprintf (dump_file, "\n=> ");
+ print_rtl (dump_file, retval);
+ }
+
+ end_sequence ();
+
+ /* Make sure this is in the instruction stream. */
+ gcc_checking_assert (PREV_INSN (note));
+ emit_insn_before (insns, note);
+ gcc_checking_assert (!NOTE_VAR_LOCATION (note));
+
+ NOTE_VAR_LOCATION (note) = retval;
+ def_expansion_record_recent (exp, retval);
+
+ return retval;
+ }
ssa_name = exp;
decl_rtl = get_rtx_for_ssa_name (ssa_name);
Index: gcc/expr.h
===================================================================
--- gcc/expr.h.orig 2011-06-02 02:16:06.332420537 -0300
+++ gcc/expr.h 2011-06-02 16:43:13.254766180 -0300
@@ -693,4 +693,11 @@ extern tree build_libfunc_function (cons
/* Get the personality libfunc for a function decl. */
rtx get_personality_function (tree);
+/* In cfgexpand.c. */
+rtx *def_get_expansion_ptr (tree);
+tree def_expansion_recent (rtx);
+void def_expansion_record_recent (tree, rtx);
+void def_expansion_reset_recent (void);
+void def_expansion_add_insns (tree, rtx, rtx);
+
#endif /* GCC_EXPR_H */
Index: gcc/explow.c
===================================================================
--- gcc/explow.c.orig 2011-06-01 20:39:58.896953301 -0300
+++ gcc/explow.c 2011-06-02 16:43:13.352765648 -0300
@@ -638,22 +638,62 @@ copy_to_mode_reg (enum machine_mode mode
return temp;
}
+/* If X is the value of a recently-expanded SSA def, emit the insns
+ generated by FN (MODE, X) at the end of the expansion of that def,
+ otherwise emit them in the current insn seq. */
+
+static inline rtx
+force_recent (enum machine_mode mode, rtx x,
+ rtx (*fn) (enum machine_mode, rtx))
+{
+ tree def = def_expansion_recent (x);
+ rtx retval;
+ rtx insns;
+
+ if (!def)
+ return fn (mode, x);
+
+ start_sequence ();
+ retval = fn (mode, x);
+ insns = get_insns ();
+ end_sequence ();
+
+ def_expansion_add_insns (def, insns, retval);
+
+ return retval;
+}
+
+static rtx force_reg_1 (enum machine_mode, rtx);
+
/* Load X into a register if it is not already one.
Use mode MODE for the register.
X should be valid for mode MODE, but it may be a constant which
is valid for all integer modes; that's why caller must specify MODE.
+ If X is the value of a recent SSA def expansion, emit the insns at
+ the end of that expansion, rather than into the current seq.
+
The caller must not alter the value in the register we return,
since we mark it as a "constant" register. */
rtx
force_reg (enum machine_mode mode, rtx x)
{
- rtx temp, insn, set;
-
if (REG_P (x))
return x;
+ return force_recent (mode, x, force_reg_1);
+}
+
+/* Load X into a register if it is not already one.
+ Use mode MODE for the register.
+ Internal implementation of force_reg. */
+
+static rtx
+force_reg_1 (enum machine_mode mode, rtx x)
+{
+ rtx temp, insn, set;
+
if (general_operand (x, mode))
{
temp = gen_reg_rtx (mode);
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in.orig 2011-06-01 20:39:59.074953270 -0300
+++ gcc/Makefile.in 2011-06-02 16:43:13.484764930 -0300
@@ -2942,7 +2942,8 @@ except.o : except.c $(CONFIG_H) $(SYSTEM
expr.o : expr.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \
$(TREE_H) $(FLAGS_H) $(FUNCTION_H) $(REGS_H) $(EXPR_H) $(OPTABS_H) \
$(LIBFUNCS_H) $(INSN_ATTR_H) insn-config.h $(RECOG_H) output.h \
- typeclass.h hard-reg-set.h toplev.h $(DIAGNOSTIC_CORE_H) hard-reg-set.h $(EXCEPT_H) \
+ typeclass.h hard-reg-set.h toplev.h $(DIAGNOSTIC_CORE_H) \
+ gimple-pretty-print.h hard-reg-set.h $(EXCEPT_H) \
reload.h langhooks.h intl.h $(TM_P_H) $(TARGET_H) \
tree-iterator.h gt-expr.h $(MACHMODE_H) $(TIMEVAR_H) $(TREE_FLOW_H) \
$(TREE_PASS_H) $(DF_H) $(DIAGNOSTIC_H) vecprim.h $(SSAEXPAND_H)
[-- Attachment #3: Type: text/plain, Size: 258 bytes --]
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2011-06-03 1:21 ` Alexandre Oliva
@ 2012-04-09 6:08 ` Alexandre Oliva
2012-06-13 8:09 ` Alexandre Oliva
0 siblings, 1 reply; 12+ messages in thread
From: Alexandre Oliva @ 2012-04-09 6:08 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]
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?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: expand-replaceable-for-debug-only-pr48866.patch --]
[-- Type: text/x-diff, Size: 17992 bytes --]
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))
[-- Attachment #3: Type: text/plain, Size: 258 bytes --]
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PR48866] three alternative fixes
2012-04-09 6:08 ` Alexandre Oliva
@ 2012-06-13 8:09 ` Alexandre Oliva
0 siblings, 0 replies; 12+ messages in thread
From: Alexandre Oliva @ 2012-06-13 8:09 UTC (permalink / raw)
To: gcc-patches
On Apr 9, 2012, Alexandre Oliva <aoliva@redhat.com> wrote:
> 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.
Ping? http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00413.html
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-06-13 8:08 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-30 13:25 [PR48866] three alternative fixes Alexandre Oliva
2011-05-30 13:34 ` Alexandre Oliva
2011-06-03 1:21 ` Alexandre Oliva
2012-04-09 6:08 ` Alexandre Oliva
2012-06-13 8:09 ` Alexandre Oliva
2011-05-30 14:30 ` Alexandre Oliva
2011-06-03 1:28 ` Alexandre Oliva
2011-06-03 1:37 ` Alexandre Oliva
2011-05-30 14:55 ` Alexandre Oliva
2011-05-30 15:35 ` Michael Matz
2011-06-01 22:31 ` Alexandre Oliva
2011-06-03 1:34 ` Alexandre Oliva
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).