Re: [PATCH] Remove "note: " prefix from some scan-tree-dump directives

[Christophe Lyon <christophe dot lyon at linaro dot org>]

On Tue, 3 Jul 2018 at 16:11, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Tue, Jul 3, 2018 at 4:10 PM David Malcolm <dmalcolm@redhat.com> wrote:
> >
> > On Tue, 2018-07-03 at 15:53 +0200, Richard Biener wrote:
> > > On Tue, Jul 3, 2018 at 3:52 PM David Malcolm <dmalcolm@redhat.com>
> > > wrote:
> > > >
> > > > On Tue, 2018-07-03 at 09:37 +0200, Richard Biener wrote:
> > > > > On Mon, Jul 2, 2018 at 7:00 PM David Malcolm <dmalcolm@redhat.com
> > > > > >
> > > > > wrote:
> > > > > >
> > > > > > On Mon, 2018-07-02 at 14:23 +0200, Christophe Lyon wrote:
> > > > > > > On Fri, 29 Jun 2018 at 10:09, Richard Biener <richard.guenthe
> > > > > > > r@gm
> > > > > > > ail.
> > > > > > > com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jun 26, 2018 at 5:43 PM David Malcolm <dmalcolm@red
> > > > > > > > hat.
> > > > > > > > com>
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > This patch adds a concept of nested "scopes" to
> > > > > > > > > dumpfile.c's
> > > > > > > > > dump_*_loc
> > > > > > > > > calls, and wires it up to the DUMP_VECT_SCOPE macro in
> > > > > > > > > tree-
> > > > > > > > > vectorizer.h,
> > > > > > > > > so that the nested structure is shown in -fopt-info by
> > > > > > > > > indentation.
> > > > > > > > >
> > > > > > > > > For example, this converts -fopt-info-all e.g. from:
> > > > > > > > >
> > > > > > > > > test.c:8:3: note: === analyzing loop ===
> > > > > > > > > test.c:8:3: note: === analyze_loop_nest ===
> > > > > > > > > test.c:8:3: note: === vect_analyze_loop_form ===
> > > > > > > > > test.c:8:3: note: === get_loop_niters ===
> > > > > > > > > test.c:8:3: note: symbolic number of iterations is
> > > > > > > > > (unsigned
> > > > > > > > > int)
> > > > > > > > > n_9(D)
> > > > > > > > > test.c:8:3: note: not vectorized: loop contains function
> > > > > > > > > calls or
> > > > > > > > > data references that cannot be analyzed
> > > > > > > > > test.c:8:3: note: vectorized 0 loops in function
> > > > > > > > >
> > > > > > > > > to:
> > > > > > > > >
> > > > > > > > > test.c:8:3: note: === analyzing loop ===
> > > > > > > > > test.c:8:3: note:  === analyze_loop_nest ===
> > > > > > > > > test.c:8:3: note:   === vect_analyze_loop_form ===
> > > > > > > > > test.c:8:3: note:    === get_loop_niters ===
> > > > > > > > > test.c:8:3: note:   symbolic number of iterations is
> > > > > > > > > (unsigned
> > > > > > > > > int) n_9(D)
> > > > > > > > > test.c:8:3: note:   not vectorized: loop contains
> > > > > > > > > function
> > > > > > > > > calls
> > > > > > > > > or data references that cannot be analyzed
> > > > > > > > > test.c:8:3: note: vectorized 0 loops in function
> > > > > > > > >
> > > > > > > > > showing that the "symbolic number of iterations" message
> > > > > > > > > is
> > > > > > > > > within
> > > > > > > > > the "=== analyze_loop_nest ===" (and not within the
> > > > > > > > > "=== vect_analyze_loop_form ===").
> > > > > > > > >
> > > > > > > > > This is also enabling work for followups involving
> > > > > > > > > optimization
> > > > > > > > > records
> > > > > > > > > (allowing the records to directly capture the nested
> > > > > > > > > structure of
> > > > > > > > > the
> > > > > > > > > dump messages).
> > > > > > > > >
> > > > > > > > > Successfully bootstrapped & regrtested on x86_64-pc-
> > > > > > > > > linux-
> > > > > > > > > gnu.
> > > > > > > > >
> > > > > > > > > OK for trunk?
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > I've noticed that this patch (r262246) caused regressions on
> > > > > > > aarch64:
> > > > > > >     gcc.dg/vect/slp-perm-1.c -flto -ffat-lto-objects  scan-
> > > > > > > tree-
> > > > > > > dump
> > > > > > > vect "note: Built SLP cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-1.c scan-tree-dump vect "note: Built
> > > > > > > SLP
> > > > > > > cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-2.c -flto -ffat-lto-objects  scan-
> > > > > > > tree-
> > > > > > > dump
> > > > > > > vect "note: Built SLP cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-2.c scan-tree-dump vect "note: Built
> > > > > > > SLP
> > > > > > > cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-3.c -flto -ffat-lto-objects  scan-
> > > > > > > tree-
> > > > > > > dump
> > > > > > > vect "note: Built SLP cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-3.c scan-tree-dump vect "note: Built
> > > > > > > SLP
> > > > > > > cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-5.c -flto -ffat-lto-objects  scan-
> > > > > > > tree-
> > > > > > > dump
> > > > > > > vect "note: Built SLP cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-5.c scan-tree-dump vect "note: Built
> > > > > > > SLP
> > > > > > > cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects  scan-
> > > > > > > tree-
> > > > > > > dump
> > > > > > > vect "note: Built SLP cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "note: Built
> > > > > > > SLP
> > > > > > > cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-7.c -flto -ffat-lto-objects  scan-
> > > > > > > tree-
> > > > > > > dump
> > > > > > > vect "note: Built SLP cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-7.c scan-tree-dump vect "note: Built
> > > > > > > SLP
> > > > > > > cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-8.c -flto -ffat-lto-objects  scan-
> > > > > > > tree-
> > > > > > > dump
> > > > > > > vect "note: Built SLP cancelled: can use load/store-lanes"
> > > > > > >     gcc.dg/vect/slp-perm-8.c scan-tree-dump vect "note: Built
> > > > > > > SLP
> > > > > > > cancelled: can use load/store-lanes"
> > > > > > >
> > > > > > > The problem is that now there are more spaces between "note:"
> > > > > > > and
> > > > > > > "Built", the attached small patch does that for slp-perm-1.c.
> > > > > >
> > > > > > Sorry about the breakage.
> > > > > >
> > > > > > > Is it the right way of fixing it or do we want to accept any
> > > > > > > amount
> > > > > > > of
> > > > > > > spaces for instance?
> > > > > >
> > > > > > I don't think we want to hardcode the amount of space in the
> > > > > > dumpfile.
> > > > > > The idea of my patch was to make the dump more human-readable
> > > > > > (I
> > > > > > hope)
> > > > > > by visualizing the nesting structure of the dump messages, but
> > > > > > I
> > > > > > think
> > > > > > we shouldn't "bake" that into the expected strings, as someone
> > > > > > might
> > > > > > want to add an intermediate nesting level.
> > > > > >
> > > > > > Do we really need to look for the "note:" in the scan-tree-
> > > > > > dump?
> > > > > > Should that directive be rewritten to:
> > > > > >
> > > > > > -/* { dg-final { scan-tree-dump "note: Built SLP cancelled: can
> > > > > > use
> > > > > > load/store-lanes" "vect" { target { vect_perm3_int &&
> > > > > > vect_load_lanes } } } } */
> > > > > > +/* { dg-final { scan-tree-dump "Built SLP cancelled: can use
> > > > > > load/store-lanes" "vect" { target { vect_perm3_int &&
> > > > > > vect_load_lanes } } } } */
> > > > > >
> > > > > > which I believe would match any amount of spaces.
> > > > > >
> > > > > > Alternatively a regex accepting any amount of space ought to
> > > > > > work,
> > > > > > if
> > > > > > we care that the message begins with "note: ".
> > > > > >
> > > > > > The "note: " comes from dumpfile.c's dump_loc, and is emitted
> > > > > > regardless of whether it's a MSG_NOTE vs a
> > > > > > MSG_MISSED_OPTIMIZATION
> > > > > > or
> > > > > > whatever.  Given that, maybe we should just drop the "note: "
> > > > > > prefix
> > > > > > from these scan-tree-dump expected regexes?  (assuming that
> > > > > > works)
> > > > >
> > > > > I guess it was done for the -fopt-info output channel to match
> > > > > what
> > > > > we emit with inform () given those are neither warnings nor
> > > > > errors.
> > > > >
> > > > > But yes, lets elide "note: " from dumpfiles.  Be prepared to
> > > > > fiddle
> > > > > with expected scan-dumps though.
> > > >
> > > > Re-reading, I think I was unclear, but I was proposing removing it
> > > > from
> > > > the scan-tree-dump regexes, *not* from dumpfile.c (though I'm open
> > > > to
> > > > the latter).
> > >
> > > Both is fine with me.
> > >
> > > Richard.
> > >
> > > > >
> > > > > > > I'm surprised there is such little impact on the testsuite
> > > > > > > though.
> > > > > >
> > > > > > I see lots of scan-tree-dump* directives in the vect part of
> > > > > > the
> > > > > > testsuite, but it seems that only these ones use the "note: "
> > > > > > prefix; I
> > > > > > think everything else was matching against the message whilst
> > > > > > ignoring
> > > > > > the prefix, so it didn't matter when the prefix changed
> > > > > > (I double-checked and these scan-tree-dump directives didn't
> > > > > > trigger on
> > > > > > my x86_64 testing of the patch, due to { target {
> > > > > > vect_perm3_int &&
> > > > > > vect_load_lanes } }, where I see
> > > > > > check_effective_target_vect_load_lanes: returning 0 in the log)
> > > > > >
> > > > > > > If OK, I'll update the patch to take the other slp-perm-
> > > > > > > [235678].c
> > > > > > > tests into account.
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Christophe
> > > > > >
> > > > > > Sorry again about the breakage.
> > > > > > Dave
> >
> > Here's a minimal patch to remove the "note: " prefix from the
> > affected directives.
> >
> > Christophe, does this fix the issues seen on aarch64?
> >
> > OK for trunk?
>
> OK.
>
Hi David,

Sorry I missed your email: of course it works, thanks!


> Richard.
>
> > gcc/testsuite/ChangeLog:
> >         * gcc.dg/vect/slp-perm-1.c: Remove "note: " prefix from
> >         scan-tree-dump directive.
> >         * gcc.dg/vect/slp-perm-2.c: Likewise.
> >         * gcc.dg/vect/slp-perm-3.c: Likewise.
> >         * gcc.dg/vect/slp-perm-5.c: Likewise.
> >         * gcc.dg/vect/slp-perm-6.c: Likewise.
> >         * gcc.dg/vect/slp-perm-7.c: Likewise.
> >         * gcc.dg/vect/slp-perm-8.c: Likewise.
> > ---
> >  gcc/testsuite/gcc.dg/vect/slp-perm-1.c | 2 +-
> >  gcc/testsuite/gcc.dg/vect/slp-perm-2.c | 2 +-
> >  gcc/testsuite/gcc.dg/vect/slp-perm-3.c | 2 +-
> >  gcc/testsuite/gcc.dg/vect/slp-perm-5.c | 2 +-
> >  gcc/testsuite/gcc.dg/vect/slp-perm-6.c | 2 +-
> >  gcc/testsuite/gcc.dg/vect/slp-perm-7.c | 2 +-
> >  gcc/testsuite/gcc.dg/vect/slp-perm-8.c | 2 +-
> >  7 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/gcc/testsuite/gcc.dg/vect/slp-perm-1.c b/gcc/testsuite/gcc.dg/vect/slp-perm-1.c
> > index 6bd16ef..ca7803e 100644
> > --- a/gcc/testsuite/gcc.dg/vect/slp-perm-1.c
> > +++ b/gcc/testsuite/gcc.dg/vect/slp-perm-1.c
> > @@ -82,7 +82,7 @@ int main (int argc, const char* argv[])
> >  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect"  { target vect_perm } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { vect_perm3_int && {! vect_load_lanes } } } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect" { target vect_load_lanes } } } */
> > -/* { dg-final { scan-tree-dump "note: Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_int && vect_load_lanes } } } } */
> > +/* { dg-final { scan-tree-dump "Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_int && vect_load_lanes } } } } */
> >  /* { dg-final { scan-tree-dump "LOAD_LANES" "vect" { target vect_load_lanes } } } */
> >  /* { dg-final { scan-tree-dump "STORE_LANES" "vect" { target vect_load_lanes } } } */
> >
> > diff --git a/gcc/testsuite/gcc.dg/vect/slp-perm-2.c b/gcc/testsuite/gcc.dg/vect/slp-perm-2.c
> > index 4bab348..82776f3 100644
> > --- a/gcc/testsuite/gcc.dg/vect/slp-perm-2.c
> > +++ b/gcc/testsuite/gcc.dg/vect/slp-perm-2.c
> > @@ -56,6 +56,6 @@ int main (int argc, const char* argv[])
> >  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect"  { target vect_perm } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { vect_perm && {! vect_load_lanes } } } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect" { target vect_load_lanes } } } */
> > -/* { dg-final { scan-tree-dump "note: Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm && vect_load_lanes } } } } */
> > +/* { dg-final { scan-tree-dump "Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm && vect_load_lanes } } } } */
> >  /* { dg-final { scan-tree-dump "LOAD_LANES" "vect" { target vect_load_lanes } } } */
> >  /* { dg-final { scan-tree-dump "STORE_LANES" "vect" { target vect_load_lanes } } } */
> > diff --git a/gcc/testsuite/gcc.dg/vect/slp-perm-3.c b/gcc/testsuite/gcc.dg/vect/slp-perm-3.c
> > index 568e400..1807275 100644
> > --- a/gcc/testsuite/gcc.dg/vect/slp-perm-3.c
> > +++ b/gcc/testsuite/gcc.dg/vect/slp-perm-3.c
> > @@ -69,7 +69,7 @@ int main (int argc, const char* argv[])
> >  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect"  { target vect_perm } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { vect_perm && {! vect_load_lanes } } } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect" { target vect_load_lanes } } } */
> > -/* { dg-final { scan-tree-dump "note: Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm && vect_load_lanes } } } } */
> > +/* { dg-final { scan-tree-dump "Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm && vect_load_lanes } } } } */
> >  /* { dg-final { scan-tree-dump "LOAD_LANES" "vect" { target vect_load_lanes } } } */
> >  /* { dg-final { scan-tree-dump "STORE_LANES" "vect" { target vect_load_lanes } } } */
> >
> > diff --git a/gcc/testsuite/gcc.dg/vect/slp-perm-5.c b/gcc/testsuite/gcc.dg/vect/slp-perm-5.c
> > index 5293913..b86a3dc 100644
> > --- a/gcc/testsuite/gcc.dg/vect/slp-perm-5.c
> > +++ b/gcc/testsuite/gcc.dg/vect/slp-perm-5.c
> > @@ -106,7 +106,7 @@ int main (int argc, const char* argv[])
> >  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect"  { target vect_perm } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 2 "vect" { target { vect_perm3_int && { ! vect_load_lanes } } } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect" { target vect_load_lanes } } } */
> > -/* { dg-final { scan-tree-dump "note: Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_int && vect_load_lanes } } } } */
> > +/* { dg-final { scan-tree-dump "Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_int && vect_load_lanes } } } } */
> >  /* { dg-final { scan-tree-dump "LOAD_LANES" "vect" { target vect_load_lanes } } } */
> >  /* { dg-final { scan-tree-dump "STORE_LANES" "vect" { target vect_load_lanes } } } */
> >
> > diff --git a/gcc/testsuite/gcc.dg/vect/slp-perm-6.c b/gcc/testsuite/gcc.dg/vect/slp-perm-6.c
> > index 4eb648a..97a0ebf 100644
> > --- a/gcc/testsuite/gcc.dg/vect/slp-perm-6.c
> > +++ b/gcc/testsuite/gcc.dg/vect/slp-perm-6.c
> > @@ -105,6 +105,6 @@ int main (int argc, const char* argv[])
> >  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect"  { target vect_perm } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 2 "vect" { target { vect_perm3_int && { ! vect_load_lanes } } } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target vect_load_lanes } } } */
> > -/* { dg-final { scan-tree-dump "note: Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_int && vect_load_lanes } } } } */
> > +/* { dg-final { scan-tree-dump "Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_int && vect_load_lanes } } } } */
> >  /* { dg-final { scan-tree-dump "LOAD_LANES" "vect" { target vect_load_lanes } } } */
> >  /* { dg-final { scan-tree-dump "STORE_LANES" "vect" { target vect_load_lanes } } } */
> > diff --git a/gcc/testsuite/gcc.dg/vect/slp-perm-7.c b/gcc/testsuite/gcc.dg/vect/slp-perm-7.c
> > index baf7f78..346411f 100644
> > --- a/gcc/testsuite/gcc.dg/vect/slp-perm-7.c
> > +++ b/gcc/testsuite/gcc.dg/vect/slp-perm-7.c
> > @@ -98,6 +98,6 @@ int main (int argc, const char* argv[])
> >  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect"  { target vect_perm } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { vect_perm3_int && { ! vect_load_lanes } } } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect" { target vect_load_lanes } } } */
> > -/* { dg-final { scan-tree-dump "note: Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_int && vect_load_lanes } } } } */
> > +/* { dg-final { scan-tree-dump "Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_int && vect_load_lanes } } } } */
> >  /* { dg-final { scan-tree-dump "LOAD_LANES" "vect" { target vect_load_lanes } } } */
> >  /* { dg-final { scan-tree-dump "STORE_LANES" "vect" { target vect_load_lanes } } } */
> > diff --git a/gcc/testsuite/gcc.dg/vect/slp-perm-8.c b/gcc/testsuite/gcc.dg/vect/slp-perm-8.c
> > index 94d4455..17aa111 100644
> > --- a/gcc/testsuite/gcc.dg/vect/slp-perm-8.c
> > +++ b/gcc/testsuite/gcc.dg/vect/slp-perm-8.c
> > @@ -62,6 +62,6 @@ int main (int argc, const char* argv[])
> >  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { vect_perm_byte } } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { vect_perm3_byte && { ! vect_load_lanes } } } } } */
> >  /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect" { target vect_load_lanes } } } */
> > -/* { dg-final { scan-tree-dump "note: Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_byte && vect_load_lanes } } } } */
> > +/* { dg-final { scan-tree-dump "Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_byte && vect_load_lanes } } } } */
> >  /* { dg-final { scan-tree-dump "LOAD_LANES" "vect" { target vect_load_lanes } } } */
> >  /* { dg-final { scan-tree-dump "STORE_LANES" "vect" { target vect_load_lanes } } } */
> > --
> > 1.8.5.3
> >