This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] -fopt-info: add indentation via DUMP_VECT_SCOPE


On Fri, 29 Jun 2018 at 10:09, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Tue, Jun 26, 2018 at 5:43 PM David Malcolm <dmalcolm@redhat.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.
Is it the right way of fixing it or do we want to accept any amount of
spaces for instance?

I'm surprised there is such little impact on the testsuite though.

If OK, I'll update the patch to take the other slp-perm-[235678].c
tests into account.

Thanks,

Christophe

> OK and sorry for the delay.
> Richard.
>
> > gcc/ChangeLog:
> >         * dumpfile.c (dump_loc): Add indentation based on scope depth.
> >         (dump_scope_depth): New variable.
> >         (get_dump_scope_depth): New function.
> >         (dump_begin_scope): New function.
> >         (dump_end_scope): New function.
> >         * dumpfile.h (get_dump_scope_depth): New declaration.
> >         (dump_begin_scope): New declaration.
> >         (dump_end_scope): New declaration.
> >         (class auto_dump_scope): New class.
> >         (AUTO_DUMP_SCOPE): New macro.
> >         * tree-vectorizer.h (DUMP_VECT_SCOPE): Reimplement in terms of
> >         AUTO_DUMP_SCOPE.
> > ---
> >  gcc/dumpfile.c        | 35 +++++++++++++++++++++++++++++++++++
> >  gcc/dumpfile.h        | 39 +++++++++++++++++++++++++++++++++++++++
> >  gcc/tree-vectorizer.h | 15 ++++++++-------
> >  3 files changed, 82 insertions(+), 7 deletions(-)
> >
> > diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> > index 122e420..190b52d 100644
> > --- a/gcc/dumpfile.c
> > +++ b/gcc/dumpfile.c
> > @@ -419,6 +419,8 @@ dump_loc (dump_flags_t dump_kind, FILE *dfile, source_location loc)
> >                   DECL_SOURCE_FILE (current_function_decl),
> >                   DECL_SOURCE_LINE (current_function_decl),
> >                   DECL_SOURCE_COLUMN (current_function_decl));
> > +      /* Indentation based on scope depth.  */
> > +      fprintf (dfile, "%*s", get_dump_scope_depth (), "");
> >      }
> >  }
> >
> > @@ -539,6 +541,39 @@ template void dump_dec (dump_flags_t, const poly_uint64 &);
> >  template void dump_dec (dump_flags_t, const poly_offset_int &);
> >  template void dump_dec (dump_flags_t, const poly_widest_int &);
> >
> > +/* The current dump scope-nesting depth.  */
> > +
> > +static int dump_scope_depth;
> > +
> > +/* Get the current dump scope-nesting depth.
> > +   For use by dump_*_loc (for showing nesting via indentation).  */
> > +
> > +unsigned int
> > +get_dump_scope_depth ()
> > +{
> > +  return dump_scope_depth;
> > +}
> > +
> > +/* Push a nested dump scope.
> > +   Print "=== NAME ===\n" to the dumpfile, if any, and to the -fopt-info
> > +   destination, if any.
> > +   Increment the scope depth.  */
> > +
> > +void
> > +dump_begin_scope (const char *name, const dump_location_t &loc)
> > +{
> > +  dump_printf_loc (MSG_NOTE, loc, "=== %s ===\n", name);
> > +  dump_scope_depth++;
> > +}
> > +
> > +/* Pop a nested dump scope.  */
> > +
> > +void
> > +dump_end_scope ()
> > +{
> > +  dump_scope_depth--;
> > +}
> > +
> >  /* Start a dump for PHASE. Store user-supplied dump flags in
> >     *FLAG_PTR.  Return the number of streams opened.  Set globals
> >     DUMP_FILE, and ALT_DUMP_FILE to point to the opened streams, and
> > diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
> > index 90d8930..89d5c11 100644
> > --- a/gcc/dumpfile.h
> > +++ b/gcc/dumpfile.h
> > @@ -456,6 +456,45 @@ dump_enabled_p (void)
> >    return (dump_file || alt_dump_file);
> >  }
> >
> > +/* Managing nested scopes, so that dumps can express the call chain
> > +   leading to a dump message.  */
> > +
> > +extern unsigned int get_dump_scope_depth ();
> > +extern void dump_begin_scope (const char *name, const dump_location_t &loc);
> > +extern void dump_end_scope ();
> > +
> > +/* Implementation detail of the AUTO_DUMP_SCOPE macro below.
> > +
> > +   A RAII-style class intended to make it easy to emit dump
> > +   information about entering and exiting a collection of nested
> > +   function calls.  */
> > +
> > +class auto_dump_scope
> > +{
> > + public:
> > +  auto_dump_scope (const char *name, dump_location_t loc)
> > +  {
> > +    if (dump_enabled_p ())
> > +      dump_begin_scope (name, loc);
> > +  }
> > +  ~auto_dump_scope ()
> > +  {
> > +    if (dump_enabled_p ())
> > +      dump_end_scope ();
> > +  }
> > +};
> > +
> > +/* A macro for calling:
> > +     dump_begin_scope (NAME, LOC);
> > +   via an RAII object, thus printing "=== MSG ===\n" to the dumpfile etc,
> > +   and then calling
> > +     dump_end_scope ();
> > +   once the object goes out of scope, thus capturing the nesting of
> > +   the scopes.  */
> > +
> > +#define AUTO_DUMP_SCOPE(NAME, LOC) \
> > +  auto_dump_scope scope (NAME, LOC)
> > +
> >  namespace gcc {
> >
> >  class dump_manager
> > diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> > index 94a0f38..a8406b3 100644
> > --- a/gcc/tree-vectorizer.h
> > +++ b/gcc/tree-vectorizer.h
> > @@ -1440,15 +1440,16 @@ vect_get_scalar_dr_size (struct data_reference *dr)
> >  /* Source location + hotness information. */
> >  extern dump_user_location_t vect_location;
> >
> > -/* If dumping is enabled, emit a MSG_NOTE at vect_location about
> > -   entering MSG within the vectorizer.  MSG should be a string literal. */
> > +/* A macro for calling:
> > +     dump_begin_scope (MSG, vect_location);
> > +   via an RAII object, thus printing "=== MSG ===\n" to the dumpfile etc,
> > +   and then calling
> > +     dump_end_scope ();
> > +   once the object goes out of scope, thus capturing the nesting of
> > +   the scopes.  */
> >
> >  #define DUMP_VECT_SCOPE(MSG) \
> > -  do {                                         \
> > -    if (dump_enabled_p ())                     \
> > -      dump_printf_loc (MSG_NOTE, vect_location, \
> > -                      "=== " MSG " ===\n");    \
> > -  } while (0)
> > +  AUTO_DUMP_SCOPE (MSG, vect_location)
> >
> >  /*-----------------------------------------------------------------*/
> >  /* Function prototypes.                                            */
> > --
> > 1.8.5.3
> >

Attachment: slp.chlog.txt
Description: Text document

Attachment: slp.patch.txt
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]