On Tue, 11 Feb 2025, Richard Biener wrote:
> 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?
Ping.
Richard.
> 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)
