On Thu, 26 Mar 2020, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase FAILs, because gimplify_body adds a GIMPLE_NOP only
> when there are no statements in the function and with -g there is a
> DEBUG_BEGIN_STMT, so it doesn't add it and due to -fno-tree-dce that never
> gets removed afterwards.  Similarly, if the body seq after gimplification
> contains some DEBUG_BEGIN_STMTs plus a single gbind, then we could behave
> differently between -g0 and -g, by using that gbind as the body in the -g0
> case and not in the -g case.
> This patch fixes that by ignoring DEBUG_BEGIN_STMTs (other debug stmts can't
> appear at this point yet thankfully) during decisions and if we pick the
> single gbind and there are DEBUG_BEGIN_STMTs next to it, it moves them into
> the gbind.
> While debugging this, I found also a bug in the gimple_seq_last_nondebug_stmt
> function, for a seq that has a single non-DEBUG_BEGIN_STMT statement
> followed by one or more DEBUG_BEGIN_STMTs it would return NULL rather than
> the first statement.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

OK.

Thanks,
Richard.

> 2020-03-26  Jakub Jelinek  <ja...@redhat.com>
> 
>       PR debug/94281
>       * gimple.h (gimple_seq_first_nondebug_stmt): New function.
>       (gimple_seq_last_nondebug_stmt): Don't return NULL if seq contains
>       a single non-debug stmt followed by one or more debug stmts.
>       * gimplify.c (gimplify_body): Use gimple_seq_first_nondebug_stmt
>       instead of gimple_seq_first_stmt, use gimple_seq_first_nondebug_stmt
>       and gimple_seq_last_nondebug_stmt instead of gimple_seq_first and
>       gimple_seq_last to check if outer_stmt gbind could be reused and
>       if yes and it is surrounded by any debug stmts, move them into the
>       gbind body.
> 
>       * g++.dg/debug/pr94281.C: New test.
> 
> --- gcc/gimple.h.jj   2020-03-25 14:34:07.696199561 +0100
> +++ gcc/gimple.h      2020-03-25 15:26:38.045052864 +0100
> @@ -4728,6 +4728,18 @@ is_gimple_debug (const gimple *gs)
>  }
>  
>  
> +/* Return the first nondebug statement in GIMPLE sequence S.  */
> +
> +static inline gimple *
> +gimple_seq_first_nondebug_stmt (gimple_seq s)
> +{
> +  gimple_seq_node n = gimple_seq_first (s);
> +  while (n && is_gimple_debug (n))
> +    n = n->next;
> +  return n;
> +}
> +
> +
>  /* Return the last nondebug statement in GIMPLE sequence S.  */
>  
>  static inline gimple *
> @@ -4737,7 +4749,7 @@ gimple_seq_last_nondebug_stmt (gimple_se
>    for (n = gimple_seq_last (s);
>         n && is_gimple_debug (n);
>         n = n->prev)
> -    if (n->prev == s)
> +    if (n == s)
>        return NULL;
>    return n;
>  }
> --- gcc/gimplify.c.jj 2020-03-25 14:34:07.722199170 +0100
> +++ gcc/gimplify.c    2020-03-25 14:41:46.447348434 +0100
> @@ -14849,7 +14849,7 @@ gimplify_body (tree fndecl, bool do_parm
>    /* Gimplify the function's body.  */
>    seq = NULL;
>    gimplify_stmt (&DECL_SAVED_TREE (fndecl), &seq);
> -  outer_stmt = gimple_seq_first_stmt (seq);
> +  outer_stmt = gimple_seq_first_nondebug_stmt (seq);
>    if (!outer_stmt)
>      {
>        outer_stmt = gimple_build_nop ();
> @@ -14859,8 +14859,37 @@ gimplify_body (tree fndecl, bool do_parm
>    /* The body must contain exactly one statement, a GIMPLE_BIND.  If this is
>       not the case, wrap everything in a GIMPLE_BIND to make it so.  */
>    if (gimple_code (outer_stmt) == GIMPLE_BIND
> -      && gimple_seq_first (seq) == gimple_seq_last (seq))
> -    outer_bind = as_a <gbind *> (outer_stmt);
> +      && (gimple_seq_first_nondebug_stmt (seq)
> +       == gimple_seq_last_nondebug_stmt (seq)))
> +    {
> +      outer_bind = as_a <gbind *> (outer_stmt);
> +      if (gimple_seq_first_stmt (seq) != outer_stmt
> +       || gimple_seq_last_stmt (seq) != outer_stmt)
> +     {
> +       /* If there are debug stmts before or after outer_stmt, move them
> +          inside of outer_bind body.  */
> +       gimple_stmt_iterator gsi = gsi_for_stmt (outer_stmt, &seq);
> +       gimple_seq second_seq = NULL;
> +       if (gimple_seq_first_stmt (seq) != outer_stmt
> +           && gimple_seq_last_stmt (seq) != outer_stmt)
> +         {
> +           second_seq = gsi_split_seq_after (gsi);
> +           gsi_remove (&gsi, false);
> +         }
> +       else if (gimple_seq_first_stmt (seq) != outer_stmt)
> +         gsi_remove (&gsi, false);
> +       else
> +         {
> +           gsi_remove (&gsi, false);
> +           second_seq = seq;
> +           seq = NULL;
> +         }
> +       gimple_seq_add_seq_without_update (&seq,
> +                                          gimple_bind_body (outer_bind));
> +       gimple_seq_add_seq_without_update (&seq, second_seq);
> +       gimple_bind_set_body (outer_bind, seq);
> +     }
> +    }
>    else
>      outer_bind = gimple_build_bind (NULL_TREE, seq, NULL);
>  
> --- gcc/testsuite/g++.dg/debug/pr94281.C.jj   2020-03-25 14:47:11.241498952 
> +0100
> +++ gcc/testsuite/g++.dg/debug/pr94281.C      2020-03-25 14:45:32.637971196 
> +0100
> @@ -0,0 +1,11 @@
> +// PR debug/94281
> +// { dg-do compile }
> +// { dg-options "-O1 -fno-tree-dce -fipa-icf -fno-tree-forwprop 
> -fcompare-debug" }
> +
> +void fn1()
> +{
> +}
> +void fn2()
> +{
> +  ;
> +}
> 
>       Jakub
> 
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Reply via email to