Re: [PR58479] introduce a param to limit debug stmts count
On Mar 18, 2014, Jakub Jelinek wrote:
>> --- a/gcc/function.c
>> +++ b/gcc/function.c
>> @@ -4498,6 +4498,8 @@ allocate_struct_function (tree fndecl, bool abstract_p)
>>
>> cfun = ggc_alloc_cleared_function ();
>>
>> + SET_BUILD_DEBUG_STMTS (cfun, flag_var_tracking_assignments);
> Dunno how this plays together with __attribute__((optimize(...))),
> I'm afraid not very well.
MAY_HAVE_DEBUG_STMTS (and MAY_HAVE_DEBUG_INSNS) used to be defined like
that, this just saves that status in a per-function data structure. I
suppose this makes it easier to take other per-function directives into
account, but I haven't done that myself.
>> + if (code == GIMPLE_DEBUG)
>> +{
>> + gcc_checking_assert (MAY_HAVE_DEBUG_STMTS);
>> + cfun->debug_stmts++;
>> +}
> I'd strongly prefer it you could move this hunk to
> gimple_build_debug_bind_stat and gimple_build_debug_source_bind_stat,
That's where I put it at first, but it didn't fix the loop unrolling
testcase, because duplicate_bb, used for loop unrolling, and other paths
use gimple_alloc instead of specific functions to allocate copies of
existing stmts.
After posting the patchset, I got a failure to compile reflect-go with
the limit set to 20 on i686. The problem was in
maybe_move_debug_stmts_to_successors, that removed debug stmts that were
in the debug_stmts vec, to be remapped later, and when they got
remapped, they were updated (even though they were removed) and SSA DEFs
ended up linking to the removed stmt, that later got GCed. Oops.
So I went back to an earlier version of that hunk, that moved the stmts
instead of removing them.
This is the fixed patch, that I'm finishing testing along with the other
two unmodified patches, with various limits. Is this ok to install?
I'm afraid I won't be able to work on this any further, because tomorrow
I'm going to fly to travel to the US for LibrePlanet, and when I return
I'm supposed to get back to glibc. If this doesn't go in as is and
nobody else picks it up, it will have to wait until I get another time
slot for GCC.
Optimize debug stmt generation after limit is exceeded
From: Alexandre Oliva
for gcc/ChangeLog
PR debug/58479
* ipa-prop.c (ipa_modify_call_arguments): Don't build new
debug stmts if BUILD_DEBUG_STMTS_P doesn't hold any more.
* ipa-split.c (split_function): Likewise.
* tree-cfg.c (gimple_merge_blocks): Likewise.
(gimple_duplicate_bb): Likewise.
* tree-inline.c (remap_ssa_name): Likewise.
(remap_gimple_seq, copy_bb): Tolerate that...
(remap_gimple_stmt): ... return NULL after the debug stmt
limit is reached. Check that debug stmts don't get to where
we don't expect them.
(maybe_move_debug_stmts_to_successors): Move trailing debug
stmts to any successor if we're past the limit. Avoid
confusing reuse of variable si.
(insert_init_debug_bind): Don't build debug stmts past the
limit.
(insert_init_stmt): Likewise.
* tree-into-ssa.c (insert_phi_nodes_for): Likewise.
(rewrite_debug_stmt_uses): Likewise.
(rewrite_stmt): Likewise.
(maybe_register_def): Likewise.
* tree-sra.c (generate_subtree_copies): Likewise.
(init_subtree_with_zero): Likewise.
(sra_modify_expr): Likewise.
(load_assign_lhs_subreplacements): Likewise.
(sra_modify_assign): Likewise.
(sra_ipa_reset_debug_stmts): Likewise.
* tree-ssa-dce.c (remove_dead_stmt): Likewise.
* tree-ssa-loop-ivopts.c (remove_unused_ivs): Likewise.
* tree-ssa.c (insert_debug_temp_for_var_def): Likewise.
---
gcc/ipa-prop.c |2 +-
gcc/ipa-split.c| 10 +-
gcc/tree-cfg.c |7 ++-
gcc/tree-inline.c | 30 +-
gcc/tree-into-ssa.c| 17 ++---
gcc/tree-sra.c | 15 +--
gcc/tree-ssa-dce.c |3 ++-
gcc/tree-ssa-loop-ivopts.c |2 +-
gcc/tree-ssa.c |3 ++-
9 files changed, 53 insertions(+), 36 deletions(-)
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 9f144fa..0d196b4 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3823,7 +3823,7 @@ ipa_modify_call_arguments (struct cgraph_edge *cs, gimple stmt,
}
vargs.quick_push (expr);
}
- if (adj->op != IPA_PARM_OP_COPY && MAY_HAVE_DEBUG_STMTS)
+ if (adj->op != IPA_PARM_OP_COPY && BUILD_DEBUG_STMTS_P)
{
unsigned int ix;
tree ddecl = NULL_TREE, origin = DECL_ORIGIN (adj->base), arg;
diff --git a/gcc/ipa-split.c b/gcc/ipa-split.c
index 38bd8836..6d9fc44 100644
--- a/gcc/ipa-split.c
+++ b/gcc/ipa-split.c
@@ -1300,11 +1300,11 @@ split_function (struct split_point *split_point)
tree ddecl;
gimple def_temp;
- /* This needs to be done even without MAY_HAVE_DEBUG_STMTS,
- otherwise if it didn't exist before, we'd end up with
- different SSA_NAME_VERSIONs between -g and -g0. */
+ /* This needs to be done even without debug stmts, otherwise
+ if it didn't exist before, we'd end up with different
+ SSA_NAME_VERSIONs between -g and -g0. */
arg
Re: [PR58479] introduce a param to limit debug stmts count
On Fri, Mar 14, 2014 at 11:45:48PM -0300, Alexandre Oliva wrote:
> This bug report had various testcases that had to do with full loop
> unrolling with non-automatic iterators and fixed boundaries, which
> resulted in duplicating debug stmts in the loop for each iteration. In
> some cases, the resulting executable code is none, but the debug stmts
> add up to millions. Just dropping them on the floor is somewhat
> undesirable, even though they're not usable for much with today's
> compiler and debugger infrastructure. I decided to introduce a param to
> limit debug stmts, so that this sort of testcase doesn't run nearly
> forever, eating up all memory while at that. This is what this patchset
> does.
To some extent this is really a big hammer approach, on the other side we
already have a precedent here, the max-vartrack-size limit, and if we have
too many debug stmts in a single function, we also most likely hit the
max-vartrack-size limit anyway.
It would be nice if for the loop unrolling we could try to do something
smarter (as I wrote in the PR, I think it would be e.g. nice to preserve
debug stmts on the first few unrolled iterations and the last one and
just say the vars are all unavailable for the middle iterations or
something, instead of dropping everything).
> --- a/gcc/function.c
> +++ b/gcc/function.c
> @@ -4498,6 +4498,8 @@ allocate_struct_function (tree fndecl, bool abstract_p)
>
>cfun = ggc_alloc_cleared_function ();
>
> + SET_BUILD_DEBUG_STMTS (cfun, flag_var_tracking_assignments);
Dunno how this plays together with __attribute__((optimize(...))),
I'm afraid not very well. E.g. if in -O0 -g compilation some function is
__attribute__((optimize(2))) then we want to have debug stmts in there,
but the above would preclude it, on the other wise in -O2 -g compilation
with __attribute__((optimize(0))) function in it, we don't want debug stmts
in there. So perhaps it needs to be updated when handling the optimize
attribute if the function doesn't have a body yet or something similar?
> @@ -121,6 +121,12 @@ gimple_alloc_stat (enum gimple_code code, unsigned
> num_ops MEM_STAT_DECL)
>size_t size;
>gimple stmt;
>
> + if (code == GIMPLE_DEBUG)
> +{
> + gcc_checking_assert (MAY_HAVE_DEBUG_STMTS);
> + cfun->debug_stmts++;
> +}
> +
>size = gimple_size (code);
>if (num_ops > 0)
> size += sizeof (tree) * (num_ops - 1);
I'd strongly prefer it you could move this hunk to
gimple_build_debug_bind_stat and gimple_build_debug_source_bind_stat,
yeah, it is duplication of it, but adding a branch for all GIMPLE allocation
doesn't look like a good idea to me.
Jakub
Re: [PR58479] introduce a param to limit debug stmts count
On Sat, Mar 15, 2014 at 5:09 AM, Mike Stump wrote: > On Mar 14, 2014, at 7:45 PM, Alexandre Oliva wrote: >> In some cases, the resulting executable code is none, but the debug stmts >> add up to millions. > > I'd like to think there is a better theoretic answer to the specific > problem... trimming random debug info I think just invites a bad experience > where people want to know what is going on and to them it just feels like a > bad compiler that just randomly messed up debug info. A user that wants > faster compilation can refrain from using -g, or use -g1? > > For example, if there truly is no code, removing all scopes that have no > instruction between the start and the end along with all the debug info that > goes with those scopes. If there is one instruction, seems tome that it > should be hard to have more than a few debug statements per instruction. If > there are more than 5, it would be curious to review each one and ask the > question, is this useful and interesting? I'd like to think there are entire > classes of useless things that can be removed with no loss to the debug > experience. I agree, this doesn't seem to be a good solution (though the ability to disable VTA per function looks good to me). If we want to limit sth then we should limit the number of debug stmts inbetween two real stmts (ok, more like the ratio of debug vs. real stmts). But then the question is which debug stmts do we retain? IMHO generating debug stmts in the first place for each initializer in an unrolled int a[1]; for (;;) a[i] = 0; is bad. That is, I question the usefulness of the fancy debug stmts we create from a dead a[12345] = 0; stmt. Can we add a -fextra-verbose-var-tracking-assignments for those? Or disable it for arrays? Richard.
Re: [PR58479] introduce a param to limit debug stmts count
On Mar 14, 2014, at 7:45 PM, Alexandre Oliva wrote: > In some cases, the resulting executable code is none, but the debug stmts > add up to millions. I’d like to think there is a better theoretic answer to the specific problem… trimming random debug info I think just invites a bad experience where people want to know what is going on and to them it just feels like a bad compiler that just randomly messed up debug info. A user that wants faster compilation can refrain from using -g, or use -g1? For example, if there truly is no code, removing all scopes that have no instruction between the start and the end along with all the debug info that goes with those scopes. If there is one instruction, seems tome that it should be hard to have more than a few debug statements per instruction. If there are more than 5, it would be curious to review each one and ask the question, is this useful and interesting? I’d like to think there are entire classes of useless things that can be removed with no loss to the debug experience.
