Re: [PATCH] Add debug counters to back threader.

[Aldy Hernandez <aldyh@redhat.com>]

On Tue, Nov 2, 2021 at 3:13 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Nov 2, 2021 at 2:36 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> > On Tue, Nov 2, 2021 at 2:27 PM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> > >
> > > On Mon, Nov 1, 2021 at 2:03 PM Jeff Law via Gcc-patches
> > > <gcc-patches@gcc.gnu.org> wrote:
> > > >
> > > >
> > > >
> > > > On 11/1/2021 3:54 AM, Aldy Hernandez wrote:
> > > > > Chasing down stage3 miscomparisons is never fun, and having no way to
> > > > > distinguish between jump threads registered by a particular
> > > > > pass, is even harder.  This patch adds debug counters for the individual
> > > > > back threading passes.  I've left the ethread pass alone, as that one is
> > > > > usually benign, but we could easily add it if needed.
> > > > >
> > > > > The fact that we can only pass one boolean argument to the passes
> > > > > infrastructure has us do all sorts of gymnastics to differentiate
> > > > > between the various back threading passes.
> > > > >
> > > > > Tested on x86-64 Linux.
> > > > >
> > > > > OK?
> > > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > >       * dbgcnt.def: Add debug counter for back_thread[12] and
> > > > >       back_threadfull[12].
> > > > >       * passes.def: Pass "first" argument to each back threading pass.
> > > > >       * tree-ssa-threadbackward.c (back_threader::back_threader): Add
> > > > >       first argument.
> > > > >       (back_threader::debug_counter): New.
> > > > >       (back_threader::maybe_register_path): Call debug_counter.
> > > > OK
> > >
> > > But it's ugly.  Very.  Why isn't a single debug-counter good enough?
> > > You should be able to reduce to a single threading pass via
> > > -fdisable-tree-xyz and then bisect with the debug counter.
> >
> > Indeed.  I'm not a big fan either.
> >
> > The -fdisable-tree-xyz approach is my first line of defense, but
> > sometimes the problem is a combination of threading passes working in
> > tandem.  For example, thread1 threads a path that causes a later
> > thread99 pass to find another path.  So I can't just have one counter.
> > We need to be able to bisect the thread1 path, and then, if there's
> > still a problem, bisect the thread99 pass.
> >
> > I was fighting a bootstrap miscomparison bug when I could reduce the
> > problem to 2 threading passes, and then further to thread1's 123 path,
> > and thread2's 567 and 890 paths.  Not fun.
>
> Btw, you can now do -fdbg-cnt=thread:5:892-11111 to continue
> bisecting the number space of thread2 after fixing '5' for thread1.

I think you meant -fdbg-cnt=thread:5-5:892-11111, since just the 5 is
1-5 AFAICT.

>
> And -fdbg-cnt-list will tell you the upper bound of the counters
> (for the 11111).
>
> But yes, not fun.  But really "bisecting" multiple counters at the same
> time doesn't work better than "bisecting" a single counter into
> multiple slices.

I find it easier to bisect using multiple counters, but if the general
consensus is that bisecting using a single counter and multiple slices
is easier, I'll switch it back.  However, I'd like to get some
feedback from the bugzilla gurus here, as they're doing most of the
heavy lifting here.

Martin, pinskia, do you have opinions here?

Aldy