Re: [SFN] Bootstrap broken

[Jakub Jelinek <jakub at redhat dot com>]

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 <aoliva@redhat.com>
> 	    Jakub Jelinek  <jakub@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