This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Remove forwarder blocks in tree-cfgcleanup even at -O0 if they don't change locations (PR debug/44205)
- From: Richard Guenther <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 21 May 2010 10:33:10 +0200 (CEST)
- Subject: Re: [PATCH] Remove forwarder blocks in tree-cfgcleanup even at -O0 if they don't change locations (PR debug/44205)
- References: <20100521081110.GQ10293@tyan-ft48-01.lab.bos.redhat.com>
On Fri, 21 May 2010, Jakub Jelinek wrote:
> Hi!
>
> The testcase from the PR has wrong line information at -O0.
> When we have if (x) { if (y) { block1; } else { block2; } } else { block3; } block4;
> then the next location after block1 lines should be block4, rather than
> block2. The gimplifier creates in this case
> ... block1; goto cont_label_after_block2; block2; cont_label_after_block2: goto cont_label_after_block3; ...
> where the gotos intentionally don't have any locus, only
> block{1,2,3,4} stmts and the condition stmts have:
> g = gimple_build_goto (label_cont);
>
> /* GIMPLE_COND's are very low level; they have embedded
> gotos. This particular embedded goto should not be marked
> with the location of the original COND_EXPR, as it would
> correspond to the COND_EXPR's condition, not the ELSE or
> the
> THEN arms. To avoid marking it with the wrong location,
> flag
> it as "no location". */
> gimple_set_do_not_emit_location (g);
> but it would be very hard at gimplify time to ensure that block1's end goto
> jumps to cont lable after block3 instead after block1.
> In tree-cfgcleanup we then refuse to remove the forwarder block after
> block2:
> /* Forwarder blocks can carry line number information which is
> useful when debugging, so we only clean them up when
> optimizing. */
> if (optimize > 0
> && tree_forwarder_block_p (bb, false)
> && remove_forwarder_block (bb))
> return true;
> At RTL expansion time, the jumps which previously had no locus get one
> from the preceeding insns and then in cfglayout mode finally the forwarder
> block is removed and the jump that is kept has line number of last stmt
> in block2.
>
> The following patch fixes it by enabling removal of forwarder blocks even
> at -O0, as long as they don't affect line numbers (locus of all
> incoming/outgoing edges and labels is the same).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux and Jan Kratochvil
> regtested it on gdb testsuite. Ok for trunk?
Ok. Is this a regression in 4.5 as we removed the useless pass
which did some of the jump optimizations in 4.4?
Thanks,
Richard.
> 2010-05-21 Jakub Jelinek <jakub@redhat.com>
>
> PR debug/44205
> * tree-cfgcleanup.c (tree_forwarder_block_p): Return false if
> at -O0 goto_locus of any of the incoming edges differs from
> goto_locus of outgoing edge, or gimple_location of any of the
> labels differs.
>
> --- gcc/tree-cfgcleanup.c.jj 2010-04-05 18:48:59.000000000 +0200
> +++ gcc/tree-cfgcleanup.c 2010-05-20 16:29:42.000000000 +0200
> @@ -267,6 +267,7 @@ static bool
> tree_forwarder_block_p (basic_block bb, bool phi_wanted)
> {
> gimple_stmt_iterator gsi;
> + location_t locus;
>
> /* BB must have a single outgoing edge. */
> if (single_succ_p (bb) != 1
> @@ -285,6 +286,8 @@ tree_forwarder_block_p (basic_block bb,
> gcc_assert (bb != ENTRY_BLOCK_PTR);
> #endif
>
> + locus = single_succ_edge (bb)->goto_locus;
> +
> /* There should not be an edge coming from entry, or an EH edge. */
> {
> edge_iterator ei;
> @@ -293,6 +296,10 @@ tree_forwarder_block_p (basic_block bb,
> FOR_EACH_EDGE (e, ei, bb->preds)
> if (e->src == ENTRY_BLOCK_PTR || (e->flags & EDGE_EH))
> return false;
> + /* If goto_locus of any of the edges differs, prevent removing
> + the forwarder block for -O0. */
> + else if (optimize == 0 && e->goto_locus != locus)
> + return false;
> }
>
> /* Now walk through the statements backward. We can ignore labels,
> @@ -306,6 +313,8 @@ tree_forwarder_block_p (basic_block bb,
> case GIMPLE_LABEL:
> if (DECL_NONLOCAL (gimple_label_label (stmt)))
> return false;
> + if (optimize == 0 && gimple_location (stmt) != locus)
> + return false;
> break;
>
> /* ??? For now, hope there's a corresponding debug
> @@ -608,11 +617,7 @@ cleanup_tree_cfg_bb (basic_block bb)
>
> retval = cleanup_control_flow_bb (bb);
>
> - /* Forwarder blocks can carry line number information which is
> - useful when debugging, so we only clean them up when
> - optimizing. */
> - if (optimize > 0
> - && tree_forwarder_block_p (bb, false)
> + if (tree_forwarder_block_p (bb, false)
> && remove_forwarder_block (bb))
> return true;
>
>
> Jakub
>
>
--
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex