On Wed, Dec 13, 2017 at 11:45:51AM +0100, Jakub Jelinek wrote:
> On Wed, Dec 13, 2017 at 10:28:22AM +0100, Jakub Jelinek wrote:
> > Formatting, this should be
> > bool can_move_early_debug_stmts
> > = ...
> > and the line is too long, so needs to be wrapped.
> >
> > Furthermore, I must say I don't understand why
> > can_move_early_debug_stmts should care whether there are any labels in
> > dest bb or not. That sounds very risky for introducing non-# DEBUG
> > BEGIN_STMT
> > debug insns before labels if it could happen. Though, if
> > gsi_stmt (gsi) == gsi_stmt (gsie), then the loop right below it will not
> > do anything and nothing cares about can_move_early_debug_stmts afterwards.
> > So, in short, can_move_early_debug_stmts is used only if
> > gsi_stmt (gsi) != gsi_stmt (gsie), and therefore
> > can_move_early_debug_stmts if it is used is can_move_debug_stmts && (1 ||
> > ...);
> >
> > So, can we get rid of can_move_early_debug_stmts altogether and just use
> > can_move_debug_stmts in there instead?
> >
> > Another thing I find risky is that you compute gsie_to so early and don't
> > update it. If you don't need it above for can_move_early_debug_stmts, can
> > you just do it back where it used to be done,
>
> Here it is everything in patch form, in case some volunteers are willing to
> test it on their targets, because we need faster turn-arounds for this.
>
> 2017-12-13 Alexandre Oliva <aol...@redhat.com>
> Jakub Jelinek <ja...@redhat.com>
>
> PR bootstrap/83396
> PR debug/83391
> * tree-cfgcleanup.c (remove_forwarder_block): Keep after
> labels debug stmts that can only appear after labels.
>
> * gcc.dg/torture/pr83396.c: New test.
> * g++.dg/torture/pr83391.C: New test.
Bootstrapped/regtested on x86_64-linux, i686-linux and powerpc64le-linux,
powerpc64-linux regtest still pending, ok for trunk?
> --- gcc/tree-cfgcleanup.c.jj 2017-12-12 09:48:26.813393301 +0100
> +++ gcc/tree-cfgcleanup.c 2017-12-13 11:39:03.373065381 +0100
> @@ -536,9 +536,14 @@ remove_forwarder_block (basic_block bb)
> defined labels and labels with an EH landing pad number to the
> new block, so that the redirection of the abnormal edges works,
> jump targets end up in a sane place and debug information for
> - labels is retained. */
> + labels is retained.
> +
> + While at that, move any debug stmts that appear before or in between
> + labels, but not those that can only appear after labels. */
> gsi_to = gsi_start_bb (dest);
> - for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
> + gsi = gsi_start_bb (bb);
> + gimple_stmt_iterator gsie = gsi_after_labels (bb);
> + while (gsi_stmt (gsi) != gsi_stmt (gsie))
> {
> tree decl;
> label = gsi_stmt (gsi);
> @@ -557,6 +562,21 @@ remove_forwarder_block (basic_block bb)
> gsi_next (&gsi);
> }
>
> + /* Move debug statements if the destination has a single predecessor. */
> + if (can_move_debug_stmts && !gsi_end_p (gsi))
> + {
> + gcc_assert (gsi_stmt (gsi) == gsi_stmt (gsie));
> + gimple_stmt_iterator gsie_to = gsi_after_labels (dest);
> + do
> + {
> + gimple *debug = gsi_stmt (gsi);
> + gcc_assert (is_gimple_debug (debug));
> + gsi_remove (&gsi, false);
> + gsi_insert_before (&gsie_to, debug, GSI_SAME_STMT);
> + }
> + while (!gsi_end_p (gsi));
> + }
> +
> bitmap_set_bit (cfgcleanup_altered_bbs, dest->index);
>
> /* Update the dominators. */
> --- gcc/testsuite/g++.dg/torture/pr83391.C.jj
> +++ gcc/testsuite/g++.dg/torture/pr83391.C
> @@ -0,0 +1,36 @@
> +// PR debug/83391
> +// { dg-do compile }
> +// { dg-options "-g" }
> +// { dg-additional-options "-mbranch-cost=1" { target { i?86-*-* x86_64-*-*
> mips*-*-* s390*-*-* avr*-*-* } } }
> +
> +unsigned char a;
> +enum E { F, G, H } b;
> +int c, d;
> +
> +void
> +foo ()
> +{
> + int e;
> + bool f;
> + E g = b;
> + while (1)
> + {
> + unsigned char h = a ? d : 0;
> + switch (g)
> + {
> + case 0:
> + f = h <= 'Z' || h >= 'a' && h <= 'z';
> + break;
> + case 1:
> + {
> + unsigned char i = h;
> + e = 0;
> + }
> + if (e || h)
> + g = H;
> + /* FALLTHRU */
> + default:
> + c = 0;
> + }
> + }
> +}
> --- gcc/testsuite/gcc.dg/torture/pr83396.c.jj
> +++ gcc/testsuite/gcc.dg/torture/pr83396.c
> @@ -0,0 +1,38 @@
> +/* PR bootstrap/83396 */
> +/* { dg-do compile } */
> +/* { dg-options "-g" } */
> +
> +int fn1 (void);
> +void fn2 (void *, const char *);
> +void fn3 (void);
> +
> +void
> +fn4 (long long x)
> +{
> + fn3 ();
> +}
> +
> +void
> +fn5 (long long x)
> +{
> + if (x)
> + fn3();
> +}
> +
> +void
> +fn6 (long long x)
> +{
> + switch (fn1 ())
> + {
> + case 0:
> + fn5 (x);
> + case 2:
> + fn2 (0, "");
> + break;
> + case 1:
> + case 3:
> + fn4(x);
> + case 5:
> + fn2 (0, "");
> + }
> +}
>
>
> Jakub
Jakub
