Re: [PATCH] Improve jump threading dump output.

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Sep 28, 2021 at 6:13 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
>
>
> On 9/28/21 6:05 PM, Richard Biener wrote:
> > On September 28, 2021 5:45:52 PM GMT+02:00, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >>
> >>
> >> On 9/28/2021 7:53 AM, Aldy Hernandez wrote:
> >>>
> >>>
> >>> On 9/28/21 3:47 PM, Jeff Law wrote:
> >>>>
> >>>>
> >>>> On 9/28/2021 3:45 AM, Aldy Hernandez wrote:
> >>>>> In analyzing PR102511, it has become abundantly clear that we need
> >>>>> better debugging aids for the jump threader solver.  Currently
> >>>>> debugging these issues is a nightmare if you're not intimately
> >>>>> familiar with the code.  This patch attempts to improve this.
> >>>>>
> >>>>> First, I'm enabling path solver dumps with TDF_THREADING. None of the
> >>>>> available TDF_* flags are a good match, and using TDF_DETAILS would
> >>>>> blow
> >>>>> up the dump file, since both threaders continually call the solver to
> >>>>> try out candidates.  This will allow dumping path solver details
> >>>>> without
> >>>>> having to resort to hacking the source.
> >>>>>
> >>>>> I am also dumping the current registered_jump_thread dbg counter used
> >>>>> by the registry, in the solver.  That way narrowing down a problematic
> >>>>> thread can then be examined by -fdump-*-threading and looking at the
> >>>>> solver details surrounding the appropriate counter (which the dbgcnt
> >>>>> also dumps to the dump file).
> >>>>>
> >>>>> You still need knowledge of the solver to debug these issues, but at
> >>>>> least now it's not entirely opaque.
> >>>>>
> >>>>> OK?
> >>>>>
> >>>>> gcc/ChangeLog:
> >>>>>
> >>>>>      * dbgcnt.c (dbg_cnt_counter): New.
> >>>>>      * dbgcnt.h (dbg_cnt_counter): New.
> >>>>>      * dumpfile.c (dump_options): Add entry for TDF_THREADING.
> >>>>>      * dumpfile.h (enum dump_flag): Add TDF_THREADING.
> >>>>>      * gimple-range-path.cc (DEBUG_SOLVER): Use TDF_THREADING.
> >>>>>      * tree-ssa-threadupdate.c (dump_jump_thread_path): Dump out
> >>>>>      debug counter.
> >>>> OK.
> >>>>
> >>>> Note we've got massive failures in the tester starting sometime
> >>>> yesterday and I suspect all the threader work.    So I'm going to
> >>>> slow down on reviews of that code as we stabilize stuff.
> >>>
> >>> Fair enough.  Let's knock those out then.
> >> So several are failing gcc.dg/loop-unswitch-3.c.
> >>
> >> This test appears to be verifying that we unswitch a test in one of the
> >> loops, which is no longer happening after the change to replace the VRP
> >> threader with the hybrid forward threader.
> >>
> >> So both the old VRP threader and the new style identify and realize a
> >> single jump thread.
> >>
> >> In the old VRP threader realization of the jump thread ends up creating
> >> nested loops.  In the new implementation we end up creating a single
> >> loop with two back edges to the header.
> >>
> >> ie, the (partial) graphs look like this
> >>
> >> OLD
> >>
> >>         1<--+
> >>         |      |
> >> +->  2     |
> >> |    /   \   |
> >> |  3     4  |
> >> +- +     +-+
> >>
> >> NEW
> >>
> >>
> >> +->  2 <-+
> >> |    /   \   |
> >> |  3     4  |
> >> +- +     +-+
> >>
> >>
> >> I wonder if we're not doing proper loop fixups or something similar
> >> after that change.  IIRC we have/had bits in the copier and CFG update
> >> code to mark the loops that need re-analysis and fixing up.
> >>
> >> Anyway, you should be able to trigger and analyze with a cross compiler.
> >>
> >> I've got to switch to my day job, but I'll pass along more as I get a
> >> chance to look at them.
> >
> > If you're stuck I'm also happy to help. Note that relying on loop fixup is almost never good because we easily lose track of loop association of info like OMP simd loops and all loop pragmas.
>
> I could absolutely use the help here.  Care to take a look?

Any hint on which target FAILs gcc.dg/loop-unswitch-3.c?

Richard.

> Aldy
>