[SFN] Bootstrap broken

[Christophe Lyon]

On 13 December 2017 at 11:45, Jakub Jelinek <jakub@redhat.com> 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.
>

Thanks for that, it certainly helps.

So, this version does restore a successful build on arm --with-mode=thumb,
but pr69102 still fails in arm mode.

As I mentioned in PR83396, the 4 patches attached there fix all the
problems I noticed.

HTH.

Christophe

> 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.
>
> --- 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