On Mon, 10 Feb 2025, Richard Biener wrote:

> On Mon, 10 Feb 2025, Richard Biener wrote:
> 
> > The following addresses the fact that we keep an excessive amount of
> > redundant DEBUG BEGIN_STMTs - in the testcase it sums up to 99.999%
> > of all stmts, sucking up compile-time in IL walks.  The patch amends
> > the GIMPLE DCE code that elides redundant DEBUG BIND stmts, also
> > pruning uninterrupted sequences of DEBUG BEGIN_STMTs, keeping only
> > the last one.
> > 
> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > 
> > For the testcase this brings down compile-time to 150% of -g0 levels
> > (and only 215 out of originally 1981380 DEBUG BEGIN_STMTs kept).
> > 
> > OK for trunk and possibly backports?
> 
> It regresses a few guality checks (and progresses one), I've looked
> only into one, g++.dg/guality/pr67192.C, where we now see
> FAIL: g++.dg/guality/pr67192.C   -O[123sg]  line 54 cnt == 15
> because the breakpoint happens in the wrong place.  But this shows
> it "works" only by accident.  The testcase is
> 
> __attribute__((noinline, noclone)) static void
> f4 (void)
> {
>   while (1) /* { dg-final { gdb-test 54 "cnt" "15" } } */
>     if (last ())
>       break;
>     else
>       do_it ();
>   do_it (); /* { dg-final { gdb-test 59 "cnt" "20" } } */
> }
> 
> and we have two BEGIN_STMTs for line 54(!) originally:
> 
>   [/space/rguenther/src/gcc/gcc/testsuite/g++.dg/guality/pr67192.C:54:3] # 
> DEBUG BEGIN_STMT
>   <D.2884>:
>   [/space/rguenther/src/gcc/gcc/testsuite/g++.dg/guality/pr67192.C:55:5] # 
> DEBUG BEGIN_STMT
> ...
>   [/space/rguenther/src/gcc/gcc/testsuite/g++.dg/guality/pr67192.C:54:3] # 
> DEBUG BEGIN_STMT
>   [/space/rguenther/src/gcc/gcc/testsuite/g++.dg/guality/pr67192.C:55:5] 
> goto <D.2884>;
> 
> and special code in make_blocks() moves the first BEGIN_STMT after
> the label, altering when we reach a breakpoint on the line.
> 
> You can see that with the first BEGIN_STMT moved the patch will elide it,
> and gdb will find the second location.
> 
> With removing only repeating BEGIN_STMT with exactly
> the same location (unfortunately with uint64_t a bitmap no longer
> works), we're "only" down to 996 BEGIN_STMTs for the testcase.
> 
> So I'm retesting the following.

Bootstrapped and tested on x86_64-unknown-linux-gnu without
regressions this time.

Alex, is this OK for trunk?

Thanks,
Richard.


> Richard.
> 
> From 38d49d3e2c0bf98e9e2a46e251ae0454b084cc8d Mon Sep 17 00:00:00 2001
> From: Richard Biener <rguent...@suse.de>
> Date: Mon, 10 Feb 2025 10:23:45 +0100
> Subject: [PATCH] middle-end/118801 - excessive redundant DEBUG BEGIN_STMT
> To: gcc-patches@gcc.gnu.org
> 
> The following addresses the fact that we keep an excessive amount of
> redundant DEBUG BEGIN_STMTs - in the testcase it sums up to 99.999%
> of all stmts, sucking up compile-time in IL walks.  The patch amends
> the GIMPLE DCE code that elides redundant DEBUG BIND stmts, also
> pruning uninterrupted sequences of DEBUG BEGIN_STMTs, keeping only
> the last of each set of DEBUG BEGIN_STMT with unique location.
> 
>       PR middle-end/118801
>       * tree-ssa-dce.cc (eliminate_unnecessary_stmts): Prune
>       sequences of uninterrupted DEBUG BEGIN_STMTs, keeping only
>       the last of a set with unique location.
> ---
>  gcc/tree-ssa-dce.cc | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
> index be21a2d0b50..461283ba858 100644
> --- a/gcc/tree-ssa-dce.cc
> +++ b/gcc/tree-ssa-dce.cc
> @@ -1508,6 +1508,7 @@ eliminate_unnecessary_stmts (bool aggressive)
>  
>        /* Remove dead statements.  */
>        auto_bitmap debug_seen;
> +      hash_set<int_hash <location_t, 0>> locs_seen;
>        for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi = psi)
>       {
>         stmt = gsi_stmt (gsi);
> @@ -1670,6 +1671,15 @@ eliminate_unnecessary_stmts (bool aggressive)
>               remove_dead_stmt (&gsi, bb, to_remove_edges);
>             continue;
>           }
> +       else if (gimple_debug_begin_stmt_p (stmt))
> +         {
> +           /* We are only keeping the last debug-begin in a series of
> +              debug-begin stmts.  */
> +           if (locs_seen.add (gimple_location (stmt)))
> +             remove_dead_stmt (&gsi, bb, to_remove_edges);
> +           continue;
> +         }
> +       locs_seen.empty ();
>         bitmap_clear (debug_seen);
>       }
>  
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Reply via email to