On Fri, Oct 26, 2012 at 04:30:41AM -0200, Alexandre Oliva wrote:
> From: Alexandre Oliva <lxol...@fsfla.org>
>
> for gcc/ChangeLog
>
> PR debug/54693
> * config/i386/i386.c (add_parameter_dependencies): Stop
> backward scan at the insn before the incoming head.
> (ix86_dependencies_evaluation_hook): Skip debug insns. Stop
> if first_arg is head.
Ok.
> Stabilize loop-unroll empty latch check WRT debug insns
>
> From: Alexandre Oliva <lxol...@fsfla.org>
>
> for gcc/ChangeLog
>
> PR debug/54693
> * loop-unroll.c (loop_exit_at_end_p): Skip debug insns.
Ok, this is obvious.
> Promote dead debug uses with immediate global replacement
>
> From: Alexandre Oliva <lxol...@fsfla.org>
>
> for gcc/ChangeLog
>
> PR debug/54551
> PR debug/54693
> * valtrack.c (dead_debug_global_find): Accept NULL dtemp.
> (dead_debug_global_insert): Return new entry.
> (dead_debug_global_replace_temp): Return early if REG is no
> longer in place, or if dtemp was already substituted.
> (dead_debug_promote_uses): Insert for all defs and replace all
> debug uses at once.
> (dead_debug_local_finish): Release used after promotion.
> (dead_debug_insert_temp): Stop if dtemp is NULL.
Ok.
> From: Alexandre Oliva <lxol...@fsfla.org>
>
> for gcc/ChangeLog
>
> PR debug/54693
> * tree-ssa-threadedge.c (thread_around_empty_block): Copy
> debug temps from predecessor before threading.
>
> for gcc/testsuite/ChangeLog
>
> PR debug/54693
> * gcc.dg/guality/pr54693.c: New.
Ok with:
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/guality/pr54693.c
> @@ -0,0 +1,30 @@
> +/* PR debug/54693 */
> +/* { dg-do run } */
> +/* { dg-options "-g" } */
> +
> +#include <stdlib.h>
> +
The #include <stdlib.h> looks useless to me, please remove it
and adjust gdb-test line accordingly.
> --- a/gcc/tree-ssa-threadedge.c
> +++ b/gcc/tree-ssa-threadedge.c
> @@ -637,6 +637,24 @@ thread_around_empty_block (edge taken_edge,
> if (!single_pred_p (bb))
> return NULL;
>
> + /* Before threading, copy DEBUG stmts from the predecessor, so that
> + we don't lose the bindings as we redirect the edges. */
> + if (MAY_HAVE_DEBUG_STMTS)
> + {
> + gsi = gsi_after_labels (bb);
> + for (gimple_stmt_iterator si = gsi_last_bb (taken_edge->src);
> + !gsi_end_p (si); gsi_prev (&si))
> + {
> + stmt = gsi_stmt (si);
> + if (!is_gimple_debug (stmt))
> + continue;
> +
> + stmt = gimple_copy (stmt);
> + /* ??? Should we drop the location of the copy? */
> + gsi_insert_before (&gsi, stmt, GSI_NEW_STMT);
Don't we always ignore gimple_location on debug_stmts anyway?
> Replace PHI nodes of dropped ivs with debug temps
>
> From: Alexandre Oliva <lxol...@fsfla.org>, Jakub Jelinek <ja...@redhat.com>
>
> for gcc/ChangeLog
>
> PR debug/54693
> * tree-ssa-loop-ivopts.c (remove_unused_ivs): Emit debug temps
> for dropped IV sets.
I'd appreciate review of somebody knowledgeable about IVopts here.
> + gimple def_temp = gimple_build_debug_bind (vexpr, comp, NULL);
Just wonder whether the above is sufficient, comp might be somewhat larger
expression than what we used to allow in debug bind stmts so far, where
usually it would be something with about the same complexity as valid GIMPLE
statements. I guess both tree-ssa-operands.c and cfgexpand.c handle it,
so perhaps it is fine as is, other option would be to split the expression
into smaller chunks using debug temporaries.
I.e. instead of adding
DEBUG D#123 => (sometype) (((someothertype) i_12 - 48) / 4)
or whatever get_computation_at returns emit
DEBUG D#124 => (someothertype) i_12
DEBUG D#125 => D#124 - 48
DEBUG D#126 => D#125 / 4
DEBUG D#123 => (sometype) D#126
If get_computation_at results are bound to say < 10 GIMPLE operations or
something similar, it might be ok as is.
> From: Alexandre Oliva <lxol...@fsfla.org>
>
> for gcc/ChangeLog
>
> PR debug/54693
> * gcc/valtrack.c (dead_debug_insert_temp): Defer rescan of
> newly-emitted debug insn.
I guess ok, but please install it separately.
Jakub
