On Tue, 2018-07-31 at 14:53 +0200, Richard Biener wrote:
> On Fri, Jul 27, 2018 at 11:48 PM David Malcolm <dmalc...@redhat.com>
> wrote:
> > 
> > With the addition of optinfo, the various dump_* calls had three
> > parts:
> > - optionally print to dump_file
> > - optionally print to alt_dump_file
> > - optionally make an optinfo_item and add it to the pending
> > optinfo,
> >   creating it for dump_*_loc calls.
> > 
> > However, this split makes it difficult to implement the formatted
> > dumps
> > later in patch kit, so as enabling work towards that, this patch
> > removes
> > the above split, so that all dumping within the dump_* API goes
> > through
> > optinfo_item.
> > 
> > In order to ensure that the dumps to dump_file and alt_dump_file
> > are
> > processed immediately (rather than being buffered within the
> > pending
> > optinfo for consolidation), this patch introduces the idea of
> > "immediate"
> > optinfo_item destinations vs "non-immediate" destinations.
> > 
> > The patch also adds selftest coverage of what's printed, and of
> > scopes.
> > 
> > This adds two allocations per dump_* call when dumping is enabled.
> > I'm assuming that this isn't a problem, as dump_enabled_p is
> > normally
> > false.  There are ways of optimizing it if it is an issue (by
> > making
> > optinfo_item instances become temporaries that borrow the
> > underlying
> > buffer), but they require nontrivial changes, so I'd prefer to
> > leave
> > that for another patch kit, if it becomes necessary.
> 
> Yeah, I guess that's OK given we can consolidate quite some calls
> after
> your patch anyways. 

We can, but FWIW my plan is to only touch the calls that I need to to
implement the  "Higher-level reporting of vectorization problems" idea
here:
   https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00446.html

where the explicit dump calls become implicit within calls to things
like:

   return opt_result::failure_at (stmt,
                                  "not vectorized: different sized vector "
                                  "types in statement, %T and %T\n",
                                  vectype, nunits_vectype);

But if you think it's worthwhile, I can do a big patch that uses these
format codes throughout.

> Using alloca + placement new would be possible
> as well I guess?

Maybe.  I think the underlying question here is "what should the
lifetimes of the optinfo_items (and their text buffers) be?"

In the initial version of the optinfo patch kit, I had optinfo_items
being created in response to the various dump_* calls, and them being
added to an optinfo (which takes ownership of them), before the optinfo
is eventually emitted to various destinations; the optinfo is then
deleted, deleting the owned items.

This lifetime approach (having the optinfos own the optinfo_items) was
necessary because one of the destinations was through the diagnostics
system; they needed consolidation so that all of the items could be
alive at the point of emission.  (I think the JSON output also required
it at one point).

Hence the above approach needs the items and thus their underlying text
strings to live as long as the optinfo that owns them - the
destinations assume that the optinfo_items are all alive at the point
of emission.

Hence this requires new/delete pairs for the items, and also the
xstrdup around the text buffer, so that the items can own a copy.

But the dump_file and alt_dump_file destinations don't need the items
to be long-lived: they can be temporary wrappers.

Similarly, the optimization record destination could simply work in
terms of temporary items: when an optinfo_item is added, the
corresponding JSON could be added immediately.

So I think the only things that are requiring optinfo_items to be long-
lived are:
* the -fremarks idea from an earlier patch kit - and I'm not sure what
our plans for that should be, in terms of how it should interact with
alt_dump_file/-fopt-info
* the selftests within dumpfile.c itself.

So the other approach would be to rewrite dumpfile.c so that
optinfo_item instances (or maybe "dump_item" instances) are implicitly
temporary wrappers around a text buffer; the various emit destinations
make no assumptions that the items will stick around; any that do need 
them to (e.g. for dumpfile.c's selftests) make a copy, perhaps with a
optinfo_items_need_saving_p () function to guard adding a copy of each
item into the optinfo.

That would avoid the new/delete pair for all of the optinfo_item
instances, and the xstrdup for each one, apart from during selftests.

But it's a rewrite of this code (and has interactions with the rest of
the kit, which is why I didn't do it).

Is this something you'd want me to pursue as a followup?  (it's an
optimization of the dump_enabled_p branch.  Maybe it might become more
necessary for people using -fdump-optimization-record on large
codebases???)

> OK.

Thanks
Dave


> Richard.
> 
> > gcc/ChangeLog:
> >         * dump-context.h: Include "pretty-print.h".
> >         (dump_context::refresh_dumps_are_enabled): New decl.
> >         (dump_context::emit_item): New decl.
> >         (class dump_context): Add fields "m_test_pp" and
> >         "m_test_pp_flags".
> >         (temp_dump_context::temp_dump_context): Add param
> > "test_pp_flags".
> >         (temp_dump_context::get_dumped_text): New decl.
> >         (class temp_dump_context): Add field "m_pp".
> >         * dumpfile.c (refresh_dumps_are_enabled): Convert to...
> >         (dump_context::refresh_dumps_are_enabled): ...and add a
> > test for
> >         m_test_pp.
> >         (set_dump_file): Update for above change.
> >         (set_alt_dump_file): Likewise.
> >         (dump_loc): New overload, taking a pretty_printer *.
> >         (dump_context::dump_loc): Call end_any_optinfo.  Dump the
> > location
> >         to any test pretty-printer.
> >         (make_item_for_dump_gimple_stmt): New function, adapted
> > from
> >         optinfo::add_gimple_stmt.
> >         (dump_context::dump_gimple_stmt): Call it, and use the
> > result,
> >         eliminating the direct usage of dump_file and alt_dump_file
> > in
> >         favor of indirectly using them via emit_item.
> >         (make_item_for_dump_gimple_expr): New function, adapted
> > from
> >         optinfo::add_gimple_expr.
> >         (dump_context::dump_gimple_expr): Call it, and use the
> > result,
> >         eliminating the direct usage of dump_file and alt_dump_file
> > in
> >         favor of indirectly using them via emit_item.
> >         (make_item_for_dump_generic_expr): New function, adapted
> > from
> >         optinfo::add_tree.
> >         (dump_context::dump_generic_expr): Call it, and use the
> > result,
> >         eliminating the direct usage of dump_file and alt_dump_file
> > in
> >         favor of indirectly using them via emit_item.
> >         (make_item_for_dump_printf_va): New function, adapted from
> >         optinfo::add_printf_va.
> >         (make_item_for_dump_printf): New function.
> >         (dump_context::dump_printf_va): Call
> > make_item_for_dump_printf_va,
> >         and use the result, eliminating the direct usage of
> > dump_file and
> >         alt_dump_file in favor of indirectly using them via
> > emit_item.
> >         (make_item_for_dump_dec): New function.
> >         (dump_context::dump_dec): Call it, and use the result,
> >         eliminating the direct usage of dump_file and alt_dump_file
> > in
> >         favor of indirectly using them via emit_item.
> >         (make_item_for_dump_symtab_node): New function, adapted
> > from
> >         optinfo::add_symtab_node.
> >         (dump_context::dump_symtab_node): Call it, and use the
> > result,
> >         eliminating the direct usage of dump_file and alt_dump_file
> > in
> >         favor of indirectly using them via emit_item.
> >         (dump_context::begin_scope): Reimplement, avoiding direct
> > usage
> >         of dump_file and alt_dump_file in favor of indirectly using
> > them
> >         via emit_item.
> >         (dump_context::emit_item): New member function.
> >         (temp_dump_context::temp_dump_context): Add param
> > "test_pp_flags".
> >         Set up test pretty-printer on the underlying context.  Call
> >         refresh_dumps_are_enabled.
> >         (temp_dump_context::~temp_dump_context): Call
> >         refresh_dumps_are_enabled.
> >         (temp_dump_context::get_dumped_text): New member function.
> >         (selftest::verify_dumped_text): New function.
> >         (ASSERT_DUMPED_TEXT_EQ): New macro.
> >         (selftest::test_capture_of_dump_calls): Run all tests
> > twice, with
> >         and then without optinfo enabled.  Add uses of
> >         ASSERT_DUMPED_TEXT_EQ to all tests.  Add test of nested
> > scopes.
> >         * dumpfile.h: Update comment for the dump_* API.
> >         * optinfo-emit-json.cc
> >         (selftest::test_building_json_from_dump_calls): Update for
> > new
> >         param for temp_dump_context ctor.
> >         * optinfo.cc (optinfo_item::optinfo_item): Remove "owned"
> > param
> >         and "m_owned" field.
> >         (optinfo_item::~optinfo_item): Likewise.
> >         (optinfo::add_item): New member function.
> >         (optinfo::emit): Update comment.
> >         (optinfo::add_string): Delete.
> >         (optinfo::add_printf): Delete.
> >         (optinfo::add_printf_va): Delete.
> >         (optinfo::add_gimple_stmt): Delete.
> >         (optinfo::add_gimple_expr): Delete.
> >         (optinfo::add_tree): Delete.
> >         (optinfo::add_symtab_node): Delete.
> >         (optinfo::add_dec): Delete.
> >         * optinfo.h (class dump_context): New forward decl.
> >         (optinfo::add_item): New decl.
> >         (optinfo::add_string): Delete.
> >         (optinfo::add_printf): Delete.
> >         (optinfo::add_printf_va): Delete.
> >         (optinfo::add_gimple_stmt): Delete.
> >         (optinfo::add_gimple_expr): Delete.
> >         (optinfo::add_tree): Delete.
> >         (optinfo::add_symtab_node): Delete.
> >         (optinfo::add_dec): Delete.
> >         (optinfo::add_poly_int): Delete.
> >         (optinfo_item::optinfo_item): Remove "owned" param.
> >         (class optinfo_item): Remove field "m_owned".
> > ---
> >  gcc/dump-context.h       |  16 +-
> >  gcc/dumpfile.c           | 620 ++++++++++++++++++++++++++++++++++-
> > ------------
> >  gcc/dumpfile.h           |  34 ++-
> >  gcc/optinfo-emit-json.cc |   2 +-
> >  gcc/optinfo.cc           | 135 ++---------
> >  gcc/optinfo.h            |  38 +--
> >  6 files changed, 505 insertions(+), 340 deletions(-)
> > 
> > diff --git a/gcc/dump-context.h b/gcc/dump-context.h
> > index f6df0b4..f40ea14 100644
> > --- a/gcc/dump-context.h
> > +++ b/gcc/dump-context.h
> > @@ -22,6 +22,8 @@ along with GCC; see the file COPYING3.  If not
> > see
> >  #ifndef GCC_DUMP_CONTEXT_H
> >  #define GCC_DUMP_CONTEXT_H 1
> > 
> > +#include "pretty-print.h"
> > +
> >  /* A class for handling the various dump_* calls.
> > 
> >     In particular, this class has responsibility for consolidating
> > @@ -39,6 +41,8 @@ class dump_context
> > 
> >    ~dump_context ();
> > 
> > +  void refresh_dumps_are_enabled ();
> > +
> >    void dump_loc (dump_flags_t dump_kind, const dump_location_t
> > &loc);
> > 
> >    void dump_gimple_stmt (dump_flags_t dump_kind, dump_flags_t
> > extra_dump_flags,
> > @@ -93,6 +97,8 @@ class dump_context
> > 
> >    void end_any_optinfo ();
> > 
> > +  void emit_item (optinfo_item *item, dump_flags_t dump_kind);
> > +
> >   private:
> >    optinfo &ensure_pending_optinfo ();
> >    optinfo &begin_next_optinfo (const dump_location_t &loc);
> > @@ -108,6 +114,11 @@ class dump_context
> >       if any.  */
> >    optinfo *m_pending;
> > 
> > +  /* For use in selftests: if non-NULL, then items are to be
> > printed
> > +     to this, using the given flags.  */
> > +  pretty_printer *m_test_pp;
> > +  dump_flags_t m_test_pp_flags;
> > +
> >    /* The currently active dump_context, for use by the dump_* API
> > calls.  */
> >    static dump_context *s_current;
> > 
> > @@ -123,13 +134,16 @@ class dump_context
> >  class temp_dump_context
> >  {
> >   public:
> > -  temp_dump_context (bool forcibly_enable_optinfo);
> > +  temp_dump_context (bool forcibly_enable_optinfo,
> > +                    dump_flags_t test_pp_flags);
> >    ~temp_dump_context ();
> > 
> >    /* Support for selftests.  */
> >    optinfo *get_pending_optinfo () const { return
> > m_context.m_pending; }
> > +  const char *get_dumped_text ();
> > 
> >   private:
> > +  pretty_printer m_pp;
> >    dump_context m_context;
> >    dump_context *m_saved;
> >    bool m_saved_flag_remarks;
> > diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> > index 3c8bc38..10e9cab 100644
> > --- a/gcc/dumpfile.c
> > +++ b/gcc/dumpfile.c
> > @@ -63,15 +63,6 @@ dump_flags_t dump_flags;
> >  bool dumps_are_enabled = false;
> > 
> > 
> > -/* Update the "dumps_are_enabled" global; to be called whenever
> > dump_file
> > -   or alt_dump_file change.  */
> > -
> > -static void
> > -refresh_dumps_are_enabled ()
> > -{
> > -  dumps_are_enabled = (dump_file || alt_dump_file ||
> > optinfo_enabled_p ());
> > -}
> > -
> >  /* Set global "dump_file" to NEW_DUMP_FILE, refreshing the
> > "dumps_are_enabled"
> >     global.  */
> > 
> > @@ -80,7 +71,7 @@ set_dump_file (FILE *new_dump_file)
> >  {
> >    dumpfile_ensure_any_optinfo_are_flushed ();
> >    dump_file = new_dump_file;
> > -  refresh_dumps_are_enabled ();
> > +  dump_context::get ().refresh_dumps_are_enabled ();
> >  }
> > 
> >  /* Set "alt_dump_file" to NEW_ALT_DUMP_FILE, refreshing the
> > "dumps_are_enabled"
> > @@ -91,7 +82,7 @@ set_alt_dump_file (FILE *new_alt_dump_file)
> >  {
> >    dumpfile_ensure_any_optinfo_are_flushed ();
> >    alt_dump_file = new_alt_dump_file;
> > -  refresh_dumps_are_enabled ();
> > +  dump_context::get ().refresh_dumps_are_enabled ();
> >  }
> > 
> >  #define DUMP_FILE_INFO(suffix, swtch, dkind, num) \
> > @@ -465,6 +456,27 @@ dump_loc (dump_flags_t dump_kind, FILE *dfile,
> > source_location loc)
> >      }
> >  }
> > 
> > +/* Print source location to PP if enabled.  */
> > +
> > +static void
> > +dump_loc (dump_flags_t dump_kind, pretty_printer *pp,
> > source_location loc)
> > +{
> > +  if (dump_kind)
> > +    {
> > +      if (LOCATION_LOCUS (loc) > BUILTINS_LOCATION)
> > +       pp_printf (pp, "%s:%d:%d: note: ", LOCATION_FILE (loc),
> > +                  LOCATION_LINE (loc), LOCATION_COLUMN (loc));
> > +      else if (current_function_decl)
> > +       pp_printf (pp, "%s:%d:%d: note: ",
> > +                  DECL_SOURCE_FILE (current_function_decl),
> > +                  DECL_SOURCE_LINE (current_function_decl),
> > +                  DECL_SOURCE_COLUMN (current_function_decl));
> > +      /* Indentation based on scope depth.  */
> > +      for (unsigned i = 0; i < get_dump_scope_depth (); i++)
> > +       pp_character (pp, ' ');
> > +    }
> > +}
> > +
> >  /* Implementation of dump_context member functions.  */
> > 
> >  /* dump_context's dtor.  */
> > @@ -474,12 +486,24 @@ dump_context::~dump_context ()
> >    delete m_pending;
> >  }
> > 
> > +/* Update the "dumps_are_enabled" global; to be called whenever
> > dump_file
> > +   or alt_dump_file change, or when changing dump_context in
> > selftests.  */
> > +
> > +void
> > +dump_context::refresh_dumps_are_enabled ()
> > +{
> > +  dumps_are_enabled = (dump_file || alt_dump_file ||
> > optinfo_enabled_p ()
> > +                      || m_test_pp);
> > +}
> > +
> >  /* Print LOC to the appropriate dump destinations, given
> > DUMP_KIND.
> >     If optinfos are enabled, begin a new optinfo.  */
> > 
> >  void
> >  dump_context::dump_loc (dump_flags_t dump_kind, const
> > dump_location_t &loc)
> >  {
> > +  end_any_optinfo ();
> > +
> >    location_t srcloc = loc.get_location_t ();
> > 
> >    if (dump_file && (dump_kind & pflags))
> > @@ -488,6 +512,10 @@ dump_context::dump_loc (dump_flags_t
> > dump_kind, const dump_location_t &loc)
> >    if (alt_dump_file && (dump_kind & alt_flags))
> >      ::dump_loc (dump_kind, alt_dump_file, srcloc);
> > 
> > +  /* Support for temp_dump_context in selftests.  */
> > +  if (m_test_pp && (dump_kind & m_test_pp_flags))
> > +    ::dump_loc (dump_kind, m_test_pp, srcloc);
> > +
> >    if (optinfo_enabled_p ())
> >      {
> >        optinfo &info = begin_next_optinfo (loc);
> > @@ -495,6 +523,22 @@ dump_context::dump_loc (dump_flags_t
> > dump_kind, const dump_location_t &loc)
> >      }
> >  }
> > 
> > +/* Make an item for the given dump call, equivalent to
> > print_gimple_stmt.  */
> > +
> > +static optinfo_item *
> > +make_item_for_dump_gimple_stmt (gimple *stmt, int spc,
> > dump_flags_t dump_flags)
> > +{
> > +  pretty_printer pp;
> > +  pp_needs_newline (&pp) = true;
> > +  pp_gimple_stmt_1 (&pp, stmt, spc, dump_flags);
> > +  pp_newline (&pp);
> > +
> > +  optinfo_item *item
> > +    = new optinfo_item (OPTINFO_ITEM_KIND_GIMPLE, gimple_location
> > (stmt),
> > +                       xstrdup (pp_formatted_text (&pp)));
> > +  return item;
> > +}
> > +
> >  /* Dump gimple statement GS with SPC indentation spaces and
> >     EXTRA_DUMP_FLAGS on the dump streams if DUMP_KIND is
> > enabled.  */
> > 
> > @@ -503,18 +547,18 @@ dump_context::dump_gimple_stmt (dump_flags_t
> > dump_kind,
> >                                 dump_flags_t extra_dump_flags,
> >                                 gimple *gs, int spc)
> >  {
> > -  if (dump_file && (dump_kind & pflags))
> > -    print_gimple_stmt (dump_file, gs, spc, dump_flags |
> > extra_dump_flags);
> > -
> > -  if (alt_dump_file && (dump_kind & alt_flags))
> > -    print_gimple_stmt (alt_dump_file, gs, spc, dump_flags |
> > extra_dump_flags);
> > +  optinfo_item *item
> > +    = make_item_for_dump_gimple_stmt (gs, spc, dump_flags |
> > extra_dump_flags);
> > +  emit_item (item, dump_kind);
> > 
> >    if (optinfo_enabled_p ())
> >      {
> >        optinfo &info = ensure_pending_optinfo ();
> >        info.handle_dump_file_kind (dump_kind);
> > -      info.add_gimple_stmt (gs, spc, dump_flags |
> > extra_dump_flags);
> > +      info.add_item (item);
> >      }
> > +  else
> > +    delete item;
> >  }
> > 
> >  /* Similar to dump_gimple_stmt, except additionally print source
> > location.  */
> > @@ -529,6 +573,22 @@ dump_context::dump_gimple_stmt_loc
> > (dump_flags_t dump_kind,
> >    dump_gimple_stmt (dump_kind, extra_dump_flags, gs, spc);
> >  }
> > 
> > +/* Make an item for the given dump call, equivalent to
> > print_gimple_expr.  */
> > +
> > +static optinfo_item *
> > +make_item_for_dump_gimple_expr (gimple *stmt, int spc,
> > dump_flags_t dump_flags)
> > +{
> > +  dump_flags |= TDF_RHS_ONLY;
> > +  pretty_printer pp;
> > +  pp_needs_newline (&pp) = true;
> > +  pp_gimple_stmt_1 (&pp, stmt, spc, dump_flags);
> > +
> > +  optinfo_item *item
> > +    = new optinfo_item (OPTINFO_ITEM_KIND_GIMPLE, gimple_location
> > (stmt),
> > +                       xstrdup (pp_formatted_text (&pp)));
> > +  return item;
> > +}
> > +
> >  /* Dump gimple statement GS with SPC indentation spaces and
> >     EXTRA_DUMP_FLAGS on the dump streams if DUMP_KIND is enabled.
> >     Do not terminate with a newline or semicolon.  */
> > @@ -538,18 +598,18 @@ dump_context::dump_gimple_expr (dump_flags_t
> > dump_kind,
> >                                 dump_flags_t extra_dump_flags,
> >                                 gimple *gs, int spc)
> >  {
> > -  if (dump_file && (dump_kind & pflags))
> > -    print_gimple_expr (dump_file, gs, spc, dump_flags |
> > extra_dump_flags);
> > -
> > -  if (alt_dump_file && (dump_kind & alt_flags))
> > -    print_gimple_expr (alt_dump_file, gs, spc, dump_flags |
> > extra_dump_flags);
> > +  optinfo_item *item
> > +    = make_item_for_dump_gimple_expr (gs, spc, dump_flags |
> > extra_dump_flags);
> > +  emit_item (item, dump_kind);
> > 
> >    if (optinfo_enabled_p ())
> >      {
> >        optinfo &info = ensure_pending_optinfo ();
> >        info.handle_dump_file_kind (dump_kind);
> > -      info.add_gimple_expr (gs, spc, dump_flags |
> > extra_dump_flags);
> > +      info.add_item (item);
> >      }
> > +  else
> > +    delete item;
> >  }
> > 
> >  /* Similar to dump_gimple_expr, except additionally print source
> > location.  */
> > @@ -565,6 +625,25 @@ dump_context::dump_gimple_expr_loc
> > (dump_flags_t dump_kind,
> >    dump_gimple_expr (dump_kind, extra_dump_flags, gs, spc);
> >  }
> > 
> > +/* Make an item for the given dump call, equivalent to
> > print_generic_expr.  */
> > +
> > +static optinfo_item *
> > +make_item_for_dump_generic_expr (tree node, dump_flags_t
> > dump_flags)
> > +{
> > +  pretty_printer pp;
> > +  pp_needs_newline (&pp) = true;
> > +  pp_translate_identifiers (&pp) = false;
> > +  dump_generic_node (&pp, node, 0, dump_flags, false);
> > +
> > +  location_t loc = UNKNOWN_LOCATION;
> > +  if (EXPR_HAS_LOCATION (node))
> > +    loc = EXPR_LOCATION (node);
> > +
> > +  optinfo_item *item
> > +    = new optinfo_item (OPTINFO_ITEM_KIND_TREE, loc,
> > +                       xstrdup (pp_formatted_text (&pp)));
> > +  return item;
> > +}
> > 
> >  /* Dump expression tree T using EXTRA_DUMP_FLAGS on dump streams
> > if
> >     DUMP_KIND is enabled.  */
> > @@ -574,18 +653,18 @@ dump_context::dump_generic_expr (dump_flags_t
> > dump_kind,
> >                                  dump_flags_t extra_dump_flags,
> >                                  tree t)
> >  {
> > -  if (dump_file && (dump_kind & pflags))
> > -      print_generic_expr (dump_file, t, dump_flags |
> > extra_dump_flags);
> > -
> > -  if (alt_dump_file && (dump_kind & alt_flags))
> > -      print_generic_expr (alt_dump_file, t, dump_flags |
> > extra_dump_flags);
> > +  optinfo_item *item
> > +    = make_item_for_dump_generic_expr (t, dump_flags |
> > extra_dump_flags);
> > +  emit_item (item, dump_kind);
> > 
> >    if (optinfo_enabled_p ())
> >      {
> >        optinfo &info = ensure_pending_optinfo ();
> >        info.handle_dump_file_kind (dump_kind);
> > -      info.add_tree (t, dump_flags | extra_dump_flags);
> > +      info.add_item (item);
> >      }
> > +  else
> > +    delete item;
> >  }
> > 
> > 
> > @@ -602,36 +681,56 @@ dump_context::dump_generic_expr_loc
> > (dump_flags_t dump_kind,
> >    dump_generic_expr (dump_kind, extra_dump_flags, t);
> >  }
> > 
> > +/* Make an item for the given dump call.  */
> > +
> > +static optinfo_item *
> > +make_item_for_dump_printf_va (const char *format, va_list ap)
> > +  ATTRIBUTE_PRINTF (1, 0);
> > +
> > +static optinfo_item *
> > +make_item_for_dump_printf_va (const char *format, va_list ap)
> > +{
> > +  char *formatted_text = xvasprintf (format, ap);
> > +  optinfo_item *item
> > +    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> > +                       formatted_text);
> > +  return item;
> > +}
> > +
> > +/* Make an item for the given dump call.  */
> > +
> > +static optinfo_item *
> > +make_item_for_dump_printf (const char *format, ...)
> > +  ATTRIBUTE_PRINTF (1, 2);
> > +
> > +static optinfo_item *
> > +make_item_for_dump_printf (const char *format, ...)
> > +{
> > +  va_list ap;
> > +  va_start (ap, format);
> > +  optinfo_item *item
> > +    = make_item_for_dump_printf_va (format, ap);
> > +  va_end (ap);
> > +  return item;
> > +}
> > +
> >  /* Output a formatted message using FORMAT on appropriate dump
> > streams.  */
> > 
> >  void
> >  dump_context::dump_printf_va (dump_flags_t dump_kind, const char
> > *format,
> >                               va_list ap)
> >  {
> > -  if (dump_file && (dump_kind & pflags))
> > -    {
> > -      va_list aq;
> > -      va_copy (aq, ap);
> > -      vfprintf (dump_file, format, aq);
> > -      va_end (aq);
> > -    }
> > -
> > -  if (alt_dump_file && (dump_kind & alt_flags))
> > -    {
> > -      va_list aq;
> > -      va_copy (aq, ap);
> > -      vfprintf (alt_dump_file, format, aq);
> > -      va_end (aq);
> > -    }
> > +  optinfo_item *item = make_item_for_dump_printf_va (format, ap);
> > +  emit_item (item, dump_kind);
> > 
> >    if (optinfo_enabled_p ())
> >      {
> >        optinfo &info = ensure_pending_optinfo ();
> > -      va_list aq;
> > -      va_copy (aq, ap);
> > -      info.add_printf_va (format, aq);
> > -      va_end (aq);
> > +      info.handle_dump_file_kind (dump_kind);
> > +      info.add_item (item);
> >      }
> > +  else
> > +    delete item;
> >  }
> > 
> >  /* Similar to dump_printf, except source location is also printed,
> > and
> > @@ -646,26 +745,64 @@ dump_context::dump_printf_loc_va
> > (dump_flags_t dump_kind,
> >    dump_printf_va (dump_kind, format, ap);
> >  }
> > 
> > -/* Output VALUE in decimal to appropriate dump streams.  */
> > +/* Make an item for the given dump call, equivalent to
> > print_dec.  */
> > 
> >  template<unsigned int N, typename C>
> > -void
> > -dump_context::dump_dec (dump_flags_t dump_kind, const poly_int<N,
> > C> &value)
> > +static optinfo_item *
> > +make_item_for_dump_dec (const poly_int<N, C> &value)
> >  {
> >    STATIC_ASSERT (poly_coeff_traits<C>::signedness >= 0);
> >    signop sgn = poly_coeff_traits<C>::signedness ? SIGNED :
> > UNSIGNED;
> > -  if (dump_file && (dump_kind & pflags))
> > -    print_dec (value, dump_file, sgn);
> > 
> > -  if (alt_dump_file && (dump_kind & alt_flags))
> > -    print_dec (value, alt_dump_file, sgn);
> > +  pretty_printer pp;
> > +
> > +  if (value.is_constant ())
> > +    pp_wide_int (&pp, value.coeffs[0], sgn);
> > +  else
> > +    {
> > +      pp_character (&pp, '[');
> > +      for (unsigned int i = 0; i < N; ++i)
> > +       {
> > +         pp_wide_int (&pp, value.coeffs[i], sgn);
> > +         pp_character (&pp, i == N - 1 ? ']' : ',');
> > +       }
> > +    }
> > +
> > +  optinfo_item *item
> > +    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> > +                       xstrdup (pp_formatted_text (&pp)));
> > +  return item;
> > +}
> > +
> > +/* Output VALUE in decimal to appropriate dump streams.  */
> > +
> > +template<unsigned int N, typename C>
> > +void
> > +dump_context::dump_dec (dump_flags_t dump_kind, const poly_int<N,
> > C> &value)
> > +{
> > +  optinfo_item *item = make_item_for_dump_dec (value);
> > +  emit_item (item, dump_kind);
> > 
> >    if (optinfo_enabled_p ())
> >      {
> >        optinfo &info = ensure_pending_optinfo ();
> >        info.handle_dump_file_kind (dump_kind);
> > -      info.add_poly_int<N,C> (value);
> > +      info.add_item (item);
> >      }
> > +  else
> > +    delete item;
> > +}
> > +
> > +/* Make an item for the given dump call.  */
> > +
> > +static optinfo_item *
> > +make_item_for_dump_symtab_node (symtab_node *node)
> > +{
> > +  location_t loc = DECL_SOURCE_LOCATION (node->decl);
> > +  optinfo_item *item
> > +    = new optinfo_item (OPTINFO_ITEM_KIND_SYMTAB_NODE, loc,
> > +                       xstrdup (node->dump_name ()));
> > +  return item;
> >  }
> > 
> >  /* Output the name of NODE on appropriate dump streams.  */
> > @@ -673,18 +810,17 @@ dump_context::dump_dec (dump_flags_t
> > dump_kind, const poly_int<N, C> &value)
> >  void
> >  dump_context::dump_symtab_node (dump_flags_t dump_kind,
> > symtab_node *node)
> >  {
> > -  if (dump_file && (dump_kind & pflags))
> > -    fprintf (dump_file, "%s", node->dump_name ());
> > -
> > -  if (alt_dump_file && (dump_kind & alt_flags))
> > -    fprintf (alt_dump_file, "%s", node->dump_name ());
> > +  optinfo_item *item = make_item_for_dump_symtab_node (node);
> > +  emit_item (item, dump_kind);
> > 
> >    if (optinfo_enabled_p ())
> >      {
> >        optinfo &info = ensure_pending_optinfo ();
> >        info.handle_dump_file_kind (dump_kind);
> > -      info.add_symtab_node (node);
> > +      info.add_item (item);
> >      }
> > +  else
> > +    delete item;
> >  }
> > 
> >  /* Get the current dump scope-nesting depth.
> > @@ -705,28 +841,28 @@ dump_context::get_scope_depth () const
> >  void
> >  dump_context::begin_scope (const char *name, const dump_location_t
> > &loc)
> >  {
> > -  /* Specialcase, to avoid going through dump_printf_loc,
> > -     so that we can create a optinfo of kind
> > OPTINFO_KIND_SCOPE.  */
> > -
> >    if (dump_file)
> > -    {
> > -      ::dump_loc (MSG_NOTE, dump_file, loc.get_location_t ());
> > -      fprintf (dump_file, "=== %s ===\n", name);
> > -    }
> > +    ::dump_loc (MSG_NOTE, dump_file, loc.get_location_t ());
> > 
> >    if (alt_dump_file)
> > -    {
> > -      ::dump_loc (MSG_NOTE, alt_dump_file, loc.get_location_t ());
> > -      fprintf (alt_dump_file, "=== %s ===\n", name);
> > -    }
> > +    ::dump_loc (MSG_NOTE, alt_dump_file, loc.get_location_t ());
> > +
> > +  /* Support for temp_dump_context in selftests.  */
> > +  if (m_test_pp)
> > +    ::dump_loc (MSG_NOTE, m_test_pp, loc.get_location_t ());
> > +
> > +  optinfo_item *item = make_item_for_dump_printf ("=== %s ===\n",
> > name);
> > +  emit_item (item, MSG_NOTE);
> > 
> >    if (optinfo_enabled_p ())
> >      {
> > +      optinfo &info = begin_next_optinfo (loc);
> > +      info.m_kind = OPTINFO_KIND_SCOPE;
> > +      info.add_item (item);
> >        end_any_optinfo ();
> > -      optinfo info (loc, OPTINFO_KIND_SCOPE, current_pass);
> > -      info.add_printf ("=== %s ===", name);
> > -      info.emit ();
> >      }
> > +  else
> > +    delete item;
> > 
> >    m_scope_depth++;
> >  }
> > @@ -776,6 +912,23 @@ dump_context::end_any_optinfo ()
> >    m_pending = NULL;
> >  }
> > 
> > +/* Emit ITEM to all item destinations (those that don't require
> > +   consolidation into optinfo instances).  */
> > +
> > +void
> > +dump_context::emit_item (optinfo_item *item, dump_flags_t
> > dump_kind)
> > +{
> > +  if (dump_file && (dump_kind & pflags))
> > +    fprintf (dump_file, "%s", item->get_text ());
> > +
> > +  if (alt_dump_file && (dump_kind & alt_flags))
> > +    fprintf (alt_dump_file, "%s", item->get_text ());
> > +
> > +  /* Support for temp_dump_context in selftests.  */
> > +  if (m_test_pp && (dump_kind & m_test_pp_flags))
> > +    pp_string (m_test_pp, item->get_text ());
> > +}
> > +
> >  /* The current singleton dump_context, and its default.  */
> > 
> >  dump_context *dump_context::s_current = &dump_context::s_default;
> > @@ -1510,12 +1663,18 @@ enable_rtl_dump_file (void)
> >  /* temp_dump_context's ctor.  Temporarily override the
> > dump_context
> >     (to forcibly enable optinfo-generation).  */
> > 
> > -temp_dump_context::temp_dump_context (bool
> > forcibly_enable_optinfo)
> > +temp_dump_context::temp_dump_context (bool
> > forcibly_enable_optinfo,
> > +                                     dump_flags_t test_pp_flags)
> > +
> >  : m_context (),
> >    m_saved (&dump_context ().get ())
> >  {
> >    dump_context::s_current = &m_context;
> >    m_context.m_forcibly_enable_optinfo = forcibly_enable_optinfo;
> > +  m_context.m_test_pp = &m_pp;
> > +  m_context.m_test_pp_flags = test_pp_flags;
> > +
> > +  dump_context::get ().refresh_dumps_are_enabled ();
> >  }
> > 
> >  /* temp_dump_context's dtor.  Restore the saved dump_context.  */
> > @@ -1523,6 +1682,16 @@ temp_dump_context::temp_dump_context (bool
> > forcibly_enable_optinfo)
> >  temp_dump_context::~temp_dump_context ()
> >  {
> >    dump_context::s_current = m_saved;
> > +
> > +  dump_context::get ().refresh_dumps_are_enabled ();
> > +}
> > +
> > +/* 0-terminate the text dumped so far, and return it.  */
> > +
> > +const char *
> > +temp_dump_context::get_dumped_text ()
> > +{
> > +  return pp_formatted_text (&m_pp);
> >  }
> > 
> >  namespace selftest {
> > @@ -1561,6 +1730,29 @@ test_impl_location ()
> >  #endif
> >  }
> > 
> > +/* Verify that the text dumped so far in CONTEXT equals
> > +   EXPECTED_TEXT, using LOC for the location of any failure.
> > +   As a side-effect, the internal buffer is 0-terminated.  */
> > +
> > +static void
> > +verify_dumped_text (const location &loc,
> > +                   temp_dump_context *context,
> > +                   const char *expected_text)
> > +{
> > +  gcc_assert (context);
> > +  ASSERT_STREQ_AT (loc, context->get_dumped_text (),
> > +                  expected_text);
> > +}
> > +
> > +/* Verify that the text dumped so far in CONTEXT equals
> > +   EXPECTED_TEXT.
> > +   As a side-effect, the internal buffer is 0-terminated.  */
> > +
> > +#define ASSERT_DUMPED_TEXT_EQ(CONTEXT,
> > EXPECTED_TEXT)                  \
> > +  SELFTEST_BEGIN_STMT                                             
> >      \
> > +    verify_dumped_text (SELFTEST_LOCATION, &(CONTEXT),
> > (EXPECTED_TEXT)); \
> > +  SELFTEST_END_STMT
> > +
> >  /* Verify that ITEM has the expected values.  */
> > 
> >  static void
> > @@ -1611,116 +1803,198 @@ test_capture_of_dump_calls (const
> > line_table_case &case_)
> >    linemap_line_start (line_table, 5, 100);
> >    linemap_add (line_table, LC_LEAVE, false, NULL, 0);
> >    location_t where = linemap_position_for_column (line_table, 10);
> > +  if (where > LINE_MAP_MAX_LOCATION_WITH_COLS)
> > +    return;
> > 
> >    dump_location_t loc = dump_location_t::from_location_t (where);
> > 
> > -  /* Test of dump_printf.  */
> > -  {
> > -    temp_dump_context tmp (true);
> > -    dump_printf (MSG_NOTE, "int: %i str: %s", 42, "foo");
> > -
> > -    optinfo *info = tmp.get_pending_optinfo ();
> > -    ASSERT_TRUE (info != NULL);
> > -    ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> > -    ASSERT_EQ (info->num_items (), 1);
> > -    ASSERT_IS_TEXT (info->get_item (0), "int: 42 str: foo");
> > -  }
> > -
> > -  /* Tree, via dump_generic_expr.  */
> > -  {
> > -    temp_dump_context tmp (true);
> > -    dump_printf_loc (MSG_NOTE, loc, "test of tree: ");
> > -    dump_generic_expr (MSG_NOTE, TDF_SLIM, integer_zero_node);
> > -
> > -    optinfo *info = tmp.get_pending_optinfo ();
> > -    ASSERT_TRUE (info != NULL);
> > -    ASSERT_EQ (info->get_location_t (), where);
> > -    ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> > -    ASSERT_EQ (info->num_items (), 2);
> > -    ASSERT_IS_TEXT (info->get_item (0), "test of tree: ");
> > -    ASSERT_IS_TREE (info->get_item (1), UNKNOWN_LOCATION, "0");
> > -  }
> > -
> > -  /* Tree, via dump_generic_expr_loc.  */
> > -  {
> > -    temp_dump_context tmp (true);
> > -    dump_generic_expr_loc (MSG_NOTE, loc, TDF_SLIM,
> > integer_one_node);
> > -
> > -    optinfo *info = tmp.get_pending_optinfo ();
> > -    ASSERT_TRUE (info != NULL);
> > -    ASSERT_EQ (info->get_location_t (), where);
> > -    ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> > -    ASSERT_EQ (info->num_items (), 1);
> > -    ASSERT_IS_TREE (info->get_item (0), UNKNOWN_LOCATION, "1");
> > -  }
> > -
> > -  /* Gimple.  */
> > -  {
> > -    greturn *stmt = gimple_build_return (NULL);
> > -    gimple_set_location (stmt, where);
> > -
> > -    /* dump_gimple_stmt_loc.  */
> > -    {
> > -      temp_dump_context tmp (true);
> > -      dump_gimple_stmt_loc (MSG_NOTE, loc, TDF_SLIM, stmt, 2);
> > -
> > -      optinfo *info = tmp.get_pending_optinfo ();
> > -      ASSERT_TRUE (info != NULL);
> > -      ASSERT_EQ (info->num_items (), 1);
> > -      ASSERT_IS_GIMPLE (info->get_item (0), where, "return;\n");
> > -    }
> > -
> > -    /* dump_gimple_stmt.  */
> > -    {
> > -      temp_dump_context tmp (true);
> > -      dump_gimple_stmt (MSG_NOTE, TDF_SLIM, stmt, 2);
> > +  greturn *stmt = gimple_build_return (NULL);
> > +  gimple_set_location (stmt, where);
> > 
> > -      optinfo *info = tmp.get_pending_optinfo ();
> > -      ASSERT_TRUE (info != NULL);
> > -      ASSERT_EQ (info->num_items (), 1);
> > -      ASSERT_IS_GIMPLE (info->get_item (0), where, "return;\n");
> > -    }
> > -
> > -    /* dump_gimple_expr_loc.  */
> > +  /* Run all tests twice, with and then without optinfo enabled,
> > to ensure
> > +     that immediate destinations vs optinfo-based destinations
> > both
> > +     work, independently of each other, with no leaks.  */
> > +  for (int i = 0 ; i < 2; i++)
> >      {
> > -      temp_dump_context tmp (true);
> > -      dump_gimple_expr_loc (MSG_NOTE, loc, TDF_SLIM, stmt, 2);
> > +      bool with_optinfo = (i == 0);
> > +
> > +      /* Test of dump_printf.  */
> > +      {
> > +       temp_dump_context tmp (with_optinfo, MSG_ALL);
> > +       dump_printf (MSG_NOTE, "int: %i str: %s", 42, "foo");
> > +
> > +       ASSERT_DUMPED_TEXT_EQ (tmp, "int: 42 str: foo");
> > +       if (with_optinfo)
> > +         {
> > +           optinfo *info = tmp.get_pending_optinfo ();
> > +           ASSERT_TRUE (info != NULL);
> > +           ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> > +           ASSERT_EQ (info->num_items (), 1);
> > +           ASSERT_IS_TEXT (info->get_item (0), "int: 42 str:
> > foo");
> > +         }
> > +      }
> > +
> > +      /* Tree, via dump_generic_expr.  */
> > +      {
> > +       temp_dump_context tmp (with_optinfo, MSG_ALL);
> > +       dump_printf_loc (MSG_NOTE, loc, "test of tree: ");
> > +       dump_generic_expr (MSG_NOTE, TDF_SLIM, integer_zero_node);
> > +
> > +       ASSERT_DUMPED_TEXT_EQ (tmp, "test.txt:5:10: note: test of
> > tree: 0");
> > +       if (with_optinfo)
> > +         {
> > +           optinfo *info = tmp.get_pending_optinfo ();
> > +           ASSERT_TRUE (info != NULL);
> > +           ASSERT_EQ (info->get_location_t (), where);
> > +           ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> > +           ASSERT_EQ (info->num_items (), 2);
> > +           ASSERT_IS_TEXT (info->get_item (0), "test of tree: ");
> > +           ASSERT_IS_TREE (info->get_item (1), UNKNOWN_LOCATION,
> > "0");
> > +         }
> > +      }
> > +
> > +      /* Tree, via dump_generic_expr_loc.  */
> > +      {
> > +       temp_dump_context tmp (with_optinfo, MSG_ALL);
> > +       dump_generic_expr_loc (MSG_NOTE, loc, TDF_SLIM,
> > integer_one_node);
> > +
> > +       ASSERT_DUMPED_TEXT_EQ (tmp, "test.txt:5:10: note: 1");
> > +       if (with_optinfo)
> > +         {
> > +           optinfo *info = tmp.get_pending_optinfo ();
> > +           ASSERT_TRUE (info != NULL);
> > +           ASSERT_EQ (info->get_location_t (), where);
> > +           ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> > +           ASSERT_EQ (info->num_items (), 1);
> > +           ASSERT_IS_TREE (info->get_item (0), UNKNOWN_LOCATION,
> > "1");
> > +         }
> > +      }
> > +
> > +      /* Gimple.  */
> > +      {
> > +       /* dump_gimple_stmt_loc.  */
> > +       {
> > +         temp_dump_context tmp (with_optinfo, MSG_ALL);
> > +         dump_gimple_stmt_loc (MSG_NOTE, loc, TDF_SLIM, stmt, 2);
> > +
> > +         ASSERT_DUMPED_TEXT_EQ (tmp, "test.txt:5:10: note:
> > return;\n");
> > +         if (with_optinfo)
> > +           {
> > +             optinfo *info = tmp.get_pending_optinfo ();
> > +             ASSERT_TRUE (info != NULL);
> > +             ASSERT_EQ (info->num_items (), 1);
> > +             ASSERT_IS_GIMPLE (info->get_item (0), where,
> > "return;\n");
> > +           }
> > +       }
> > 
> > -      optinfo *info = tmp.get_pending_optinfo ();
> > -      ASSERT_TRUE (info != NULL);
> > -      ASSERT_EQ (info->num_items (), 1);
> > -      ASSERT_IS_GIMPLE (info->get_item (0), where, "return;");
> > -    }
> > +       /* dump_gimple_stmt.  */
> > +       {
> > +         temp_dump_context tmp (with_optinfo, MSG_ALL);
> > +         dump_gimple_stmt (MSG_NOTE, TDF_SLIM, stmt, 2);
> > +
> > +         ASSERT_DUMPED_TEXT_EQ (tmp, "return;\n");
> > +         if (with_optinfo)
> > +           {
> > +             optinfo *info = tmp.get_pending_optinfo ();
> > +             ASSERT_TRUE (info != NULL);
> > +             ASSERT_EQ (info->num_items (), 1);
> > +             ASSERT_IS_GIMPLE (info->get_item (0), where,
> > "return;\n");
> > +           }
> > +       }
> > 
> > -    /* dump_gimple_expr.  */
> > -    {
> > -      temp_dump_context tmp (true);
> > -      dump_gimple_expr (MSG_NOTE, TDF_SLIM, stmt, 2);
> > +       /* dump_gimple_expr_loc.  */
> > +       {
> > +         temp_dump_context tmp (with_optinfo, MSG_ALL);
> > +         dump_gimple_expr_loc (MSG_NOTE, loc, TDF_SLIM, stmt, 2);
> > +
> > +         ASSERT_DUMPED_TEXT_EQ (tmp, "test.txt:5:10: note:
> > return;");
> > +         if (with_optinfo)
> > +           {
> > +             optinfo *info = tmp.get_pending_optinfo ();
> > +             ASSERT_TRUE (info != NULL);
> > +             ASSERT_EQ (info->num_items (), 1);
> > +             ASSERT_IS_GIMPLE (info->get_item (0), where,
> > "return;");
> > +           }
> > +       }
> > 
> > -      optinfo *info = tmp.get_pending_optinfo ();
> > -      ASSERT_TRUE (info != NULL);
> > -      ASSERT_EQ (info->num_items (), 1);
> > -      ASSERT_IS_GIMPLE (info->get_item (0), where, "return;");
> > +       /* dump_gimple_expr.  */
> > +       {
> > +         temp_dump_context tmp (with_optinfo, MSG_ALL);
> > +         dump_gimple_expr (MSG_NOTE, TDF_SLIM, stmt, 2);
> > +
> > +         ASSERT_DUMPED_TEXT_EQ (tmp, "return;");
> > +         if (with_optinfo)
> > +           {
> > +             optinfo *info = tmp.get_pending_optinfo ();
> > +             ASSERT_TRUE (info != NULL);
> > +             ASSERT_EQ (info->num_items (), 1);
> > +             ASSERT_IS_GIMPLE (info->get_item (0), where,
> > "return;");
> > +           }
> > +       }
> > +      }
> > +
> > +      /* poly_int.  */
> > +      {
> > +       temp_dump_context tmp (with_optinfo, MSG_ALL);
> > +       dump_dec (MSG_NOTE, poly_int64 (42));
> > +
> > +       ASSERT_DUMPED_TEXT_EQ (tmp, "42");
> > +       if (with_optinfo)
> > +         {
> > +           optinfo *info = tmp.get_pending_optinfo ();
> > +           ASSERT_TRUE (info != NULL);
> > +           ASSERT_EQ (info->num_items (), 1);
> > +           ASSERT_IS_TEXT (info->get_item (0), "42");
> > +         }
> > +      }
> > +
> > +      /* scopes.  */
> > +      {
> > +       temp_dump_context tmp (with_optinfo, MSG_ALL);
> > +       dump_printf_loc (MSG_NOTE, stmt, "msg 1\n");
> > +       {
> > +         AUTO_DUMP_SCOPE ("outer scope", stmt);
> > +         dump_printf_loc (MSG_NOTE, stmt, "msg 2\n");
> > +         {
> > +           AUTO_DUMP_SCOPE ("middle scope", stmt);
> > +           dump_printf_loc (MSG_NOTE, stmt, "msg 3\n");
> > +           {
> > +             AUTO_DUMP_SCOPE ("inner scope", stmt);
> > +             dump_printf_loc (MSG_NOTE, stmt, "msg 4\n");
> > +           }
> > +           dump_printf_loc (MSG_NOTE, stmt, "msg 5\n");
> > +         }
> > +         dump_printf_loc (MSG_NOTE, stmt, "msg 6\n");
> > +       }
> > +       dump_printf_loc (MSG_NOTE, stmt, "msg 7\n");
> > +
> > +       ASSERT_DUMPED_TEXT_EQ (tmp,
> > +                              "test.txt:5:10: note: msg 1\n"
> > +                              "test.txt:5:10: note: === outer
> > scope ===\n"
> > +                              "test.txt:5:10: note:  msg 2\n"
> > +                              "test.txt:5:10: note:  === middle
> > scope ===\n"
> > +                              "test.txt:5:10: note:   msg 3\n"
> > +                              "test.txt:5:10: note:   === inner
> > scope ===\n"
> > +                              "test.txt:5:10: note:    msg 4\n"
> > +                              "test.txt:5:10: note:   msg 5\n"
> > +                              "test.txt:5:10: note:  msg 6\n"
> > +                              "test.txt:5:10: note: msg 7\n");
> > +       if (with_optinfo)
> > +         {
> > +           optinfo *info = tmp.get_pending_optinfo ();
> > +           ASSERT_TRUE (info != NULL);
> > +           ASSERT_EQ (info->num_items (), 1);
> > +           ASSERT_IS_TEXT (info->get_item (0), "msg 7\n");
> > +         }
> > +      }
> >      }
> > -  }
> > -
> > -  /* poly_int.  */
> > -  {
> > -    temp_dump_context tmp (true);
> > -    dump_dec (MSG_NOTE, poly_int64 (42));
> > -
> > -    optinfo *info = tmp.get_pending_optinfo ();
> > -    ASSERT_TRUE (info != NULL);
> > -    ASSERT_EQ (info->num_items (), 1);
> > -    ASSERT_IS_TEXT (info->get_item (0), "42");
> > -  }
> > 
> >    /* Verify that MSG_* affects optinfo->get_kind (); we tested
> > MSG_NOTE
> >       above.  */
> >    {
> >      /* MSG_OPTIMIZED_LOCATIONS.  */
> >      {
> > -      temp_dump_context tmp (true);
> > +      temp_dump_context tmp (true, MSG_ALL);
> >        dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc, "test");
> >        ASSERT_EQ (tmp.get_pending_optinfo ()->get_kind (),
> >                  OPTINFO_KIND_SUCCESS);
> > @@ -1728,7 +2002,7 @@ test_capture_of_dump_calls (const
> > line_table_case &case_)
> > 
> >      /* MSG_MISSED_OPTIMIZATION.  */
> >      {
> > -      temp_dump_context tmp (true);
> > +      temp_dump_context tmp (true, MSG_ALL);
> >        dump_printf_loc (MSG_MISSED_OPTIMIZATION, loc, "test");
> >        ASSERT_EQ (tmp.get_pending_optinfo ()->get_kind (),
> >                  OPTINFO_KIND_FAILURE);
> > diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
> > index 1dbe3b8..2b174e5 100644
> > --- a/gcc/dumpfile.h
> > +++ b/gcc/dumpfile.h
> > @@ -442,19 +442,27 @@ dump_enabled_p (void)
> >  }
> > 
> >  /* The following API calls (which *don't* take a "FILE *")
> > -   write the output to zero or more locations:
> > -   (a) the active dump_file, if any
> > -   (b) the -fopt-info destination, if any
> > -   (c) to the "optinfo" destinations, if any:
> > -       (c.1) as optimization records
> > -
> > -   dump_* (MSG_*) --> dumpfile.c --+--> (a) dump_file
> > -                                   |
> > -                                   +--> (b) alt_dump_file
> > -                                   |
> > -                                   `--> (c) optinfo
> > -                                            `---> optinfo
> > destinations
> > -                                                  (c.1)
> > optimization records
> > +   write the output to zero or more locations.
> > +
> > +   Some destinations are written to immediately as dump_* calls
> > +   are made; for others, the output is consolidated into an
> > "optinfo"
> > +   instance (with its own metadata), and only emitted once the
> > optinfo
> > +   is complete.
> > +
> > +   The destinations are:
> > +
> > +   (a) the "immediate" destinations:
> > +       (a.1) the active dump_file, if any
> > +       (a.2) the -fopt-info destination, if any
> > +   (b) the "optinfo" destinations, if any:
> > +       (b.1) as optimization records
> > +
> > +   dump_* (MSG_*) --> dumpfile.c --> items --> (a.1) dump_file
> > +                                       |   `-> (a.2) alt_dump_file
> > +                                       |
> > +                                       `--> (b) optinfo
> > +                                                `---> optinfo
> > destinations
> > +                                                      (b.1)
> > optimization records
> > 
> >     For optinfos, the dump_*_loc mark the beginning of an optinfo
> >     instance: all subsequent dump_* calls are consolidated into
> > diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc
> > index 2199d52..992960e 100644
> > --- a/gcc/optinfo-emit-json.cc
> > +++ b/gcc/optinfo-emit-json.cc
> > @@ -537,7 +537,7 @@ namespace selftest {
> >  static void
> >  test_building_json_from_dump_calls ()
> >  {
> > -  temp_dump_context tmp (true);
> > +  temp_dump_context tmp (true, MSG_NOTE);
> >    dump_location_t loc;
> >    dump_printf_loc (MSG_NOTE, loc, "test of tree: ");
> >    dump_generic_expr (MSG_NOTE, TDF_SLIM, integer_zero_node);
> > diff --git a/gcc/optinfo.cc b/gcc/optinfo.cc
> > index 93de9d9..b858c3c 100644
> > --- a/gcc/optinfo.cc
> > +++ b/gcc/optinfo.cc
> > @@ -34,11 +34,11 @@ along with GCC; see the file COPYING3.  If not
> > see
> >  #include "cgraph.h"
> >  #include "selftest.h"
> > 
> > -/* optinfo_item's ctor.  */
> > +/* optinfo_item's ctor.  Takes ownership of TEXT.  */
> > 
> >  optinfo_item::optinfo_item (enum optinfo_item_kind kind,
> > location_t location,
> > -                           char *text, bool owned)
> > -: m_kind (kind), m_location (location), m_text (text), m_owned
> > (owned)
> > +                           char *text)
> > +: m_kind (kind), m_location (location), m_text (text)
> >  {
> >  }
> > 
> > @@ -46,8 +46,7 @@ optinfo_item::optinfo_item (enum
> > optinfo_item_kind kind, location_t location,
> > 
> >  optinfo_item::~optinfo_item ()
> >  {
> > -  if (m_owned)
> > -    free (m_text);
> > +  free (m_text);
> >  }
> > 
> >  /* Get a string from KIND.  */
> > @@ -81,7 +80,17 @@ optinfo::~optinfo ()
> >      delete item;
> >  }
> > 
> > -/* Emit the optinfo to all of the active destinations.  */
> > +/* Add ITEM to this optinfo.  */
> > +
> > +void
> > +optinfo::add_item (optinfo_item *item)
> > +{
> > +  gcc_assert (item);
> > +  m_items.safe_push (item);
> > +}
> > +
> > +/* Emit the optinfo to all of the "non-immediate" destinations
> > +   (emission to "immediate" destinations is done by
> > emit_item).  */
> > 
> >  void
> >  optinfo::emit ()
> > @@ -103,120 +112,6 @@ optinfo::handle_dump_file_kind (dump_flags_t
> > dump_kind)
> >      m_kind = OPTINFO_KIND_NOTE;
> >  }
> > 
> > -/* Append a string literal to this optinfo.  */
> > -
> > -void
> > -optinfo::add_string (const char *str)
> > -{
> > -  optinfo_item *item
> > -    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> > -                       const_cast <char *> (str), false);
> > -  m_items.safe_push (item);
> > -}
> > -
> > -/* Append printf-formatted text to this optinfo.  */
> > -
> > -void
> > -optinfo::add_printf (const char *format, ...)
> > -{
> > -  va_list ap;
> > -  va_start (ap, format);
> > -  add_printf_va (format, ap);
> > -  va_end (ap);
> > -}
> > -
> > -/* Append printf-formatted text to this optinfo.  */
> > -
> > -void
> > -optinfo::add_printf_va (const char *format, va_list ap)
> > -{
> > -  char *formatted_text = xvasprintf (format, ap);
> > -  optinfo_item *item
> > -    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> > -                       formatted_text, true);
> > -  m_items.safe_push (item);
> > -}
> > -
> > -/* Append a gimple statement to this optinfo, equivalent to
> > -   print_gimple_stmt.  */
> > -
> > -void
> > -optinfo::add_gimple_stmt (gimple *stmt, int spc, dump_flags_t
> > dump_flags)
> > -{
> > -  pretty_printer pp;
> > -  pp_needs_newline (&pp) = true;
> > -  pp_gimple_stmt_1 (&pp, stmt, spc, dump_flags);
> > -  pp_newline (&pp);
> > -
> > -  optinfo_item *item
> > -    = new optinfo_item (OPTINFO_ITEM_KIND_GIMPLE, gimple_location
> > (stmt),
> > -                       xstrdup (pp_formatted_text (&pp)), true);
> > -  m_items.safe_push (item);
> > -}
> > -
> > -/* Append a gimple statement to this optinfo, equivalent to
> > -   print_gimple_expr.  */
> > -
> > -void
> > -optinfo::add_gimple_expr (gimple *stmt, int spc, dump_flags_t
> > dump_flags)
> > -{
> > -  dump_flags |= TDF_RHS_ONLY;
> > -  pretty_printer pp;
> > -  pp_needs_newline (&pp) = true;
> > -  pp_gimple_stmt_1 (&pp, stmt, spc, dump_flags);
> > -
> > -  optinfo_item *item
> > -    = new optinfo_item (OPTINFO_ITEM_KIND_GIMPLE, gimple_location
> > (stmt),
> > -                       xstrdup (pp_formatted_text (&pp)), true);
> > -  m_items.safe_push (item);
> > -}
> > -
> > -/* Append a tree node to this optinfo, equivalent to
> > print_generic_expr.  */
> > -
> > -void
> > -optinfo::add_tree (tree node, dump_flags_t dump_flags)
> > -{
> > -  pretty_printer pp;
> > -  pp_needs_newline (&pp) = true;
> > -  pp_translate_identifiers (&pp) = false;
> > -  dump_generic_node (&pp, node, 0, dump_flags, false);
> > -
> > -  location_t loc = UNKNOWN_LOCATION;
> > -  if (EXPR_HAS_LOCATION (node))
> > -    loc = EXPR_LOCATION (node);
> > -
> > -  optinfo_item *item
> > -    = new optinfo_item (OPTINFO_ITEM_KIND_TREE, loc,
> > -                       xstrdup (pp_formatted_text (&pp)), true);
> > -  m_items.safe_push (item);
> > -}
> > -
> > -/* Append a symbol table node to this optinfo.  */
> > -
> > -void
> > -optinfo::add_symtab_node (symtab_node *node)
> > -{
> > -  location_t loc = DECL_SOURCE_LOCATION (node->decl);
> > -  optinfo_item *item
> > -    = new optinfo_item (OPTINFO_ITEM_KIND_SYMTAB_NODE, loc,
> > -                       xstrdup (node->dump_name ()), true);
> > -  m_items.safe_push (item);
> > -}
> > -
> > -/* Append the decimal represenation of a wide_int_ref to this
> > -   optinfo.  */
> > -
> > -void
> > -optinfo::add_dec (const wide_int_ref &wi, signop sgn)
> > -{
> > -  char buf[WIDE_INT_PRINT_BUFFER_SIZE];
> > -  print_dec (wi, buf, sgn);
> > -  optinfo_item *item
> > -    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> > -                       xstrdup (buf), true);
> > -  m_items.safe_push (item);
> > -}
> > -
> >  /* Should optinfo instances be created?
> >     All creation of optinfos should be guarded by this predicate.
> >     Return true if any optinfo destinations are active.  */
> > diff --git a/gcc/optinfo.h b/gcc/optinfo.h
> > index c4cf8ad..8ac961c 100644
> > --- a/gcc/optinfo.h
> > +++ b/gcc/optinfo.h
> > @@ -92,6 +92,8 @@ enum optinfo_kind
> > 
> >  extern const char *optinfo_kind_to_string (enum optinfo_kind
> > kind);
> > 
> > +class dump_context;
> > +
> >  /* A bundle of information describing part of an optimization.  */
> > 
> >  class optinfo
> > @@ -120,41 +122,14 @@ class optinfo
> >    location_t get_location_t () const { return m_loc.get_location_t
> > (); }
> >    profile_count get_count () const { return m_loc.get_count (); }
> > 
> > +  void add_item (optinfo_item *item);
> > +
> >   private:
> >    void emit ();
> > 
> >    /* Pre-canned ways of manipulating the optinfo, for use by
> > friend class
> >       dump_context.  */
> >    void handle_dump_file_kind (dump_flags_t);
> > -  void add_string (const char *str);
> > -  void add_printf (const char *format, ...) ATTRIBUTE_PRINTF_2;
> > -  void add_printf_va (const char *format, va_list ap)
> > ATTRIBUTE_PRINTF (2, 0);
> > -  void add_gimple_stmt (gimple *stmt, int spc, dump_flags_t
> > dump_flags);
> > -  void add_gimple_expr (gimple *stmt, int spc, dump_flags_t
> > dump_flags);
> > -  void add_tree (tree node, dump_flags_t dump_flags);
> > -  void add_symtab_node (symtab_node *node);
> > -  void add_dec (const wide_int_ref &wi, signop sgn);
> > -
> > -  template<unsigned int N, typename C>
> > -  void add_poly_int (const poly_int<N, C> &value)
> > -  {
> > -    /* Compare with dump_dec (MSG_NOTE, ).  */
> > -
> > -    STATIC_ASSERT (poly_coeff_traits<C>::signedness >= 0);
> > -    signop sgn = poly_coeff_traits<C>::signedness ? SIGNED :
> > UNSIGNED;
> > -
> > -    if (value.is_constant ())
> > -      add_dec (value.coeffs[0], sgn);
> > -    else
> > -      {
> > -       add_string ("[");
> > -       for (unsigned int i = 0; i < N; ++i)
> > -         {
> > -           add_dec (value.coeffs[i], sgn);
> > -           add_string (i == N - 1 ? "]" : ",");
> > -         }
> > -      }
> > -  }
> > 
> >   private:
> >    dump_location_t m_loc;
> > @@ -179,7 +154,7 @@ class optinfo_item
> >  {
> >   public:
> >    optinfo_item (enum optinfo_item_kind kind, location_t location,
> > -               char *text, bool owned);
> > +               char *text);
> >    ~optinfo_item ();
> > 
> >    enum optinfo_item_kind get_kind () const { return m_kind; }
> > @@ -191,9 +166,8 @@ class optinfo_item
> >    enum optinfo_item_kind m_kind;
> >    location_t m_location;
> > 
> > -  /* The textual form of the item.  */
> > +  /* The textual form of the item, owned by the item.  */
> >    char *m_text;
> > -  bool m_owned;
> >  };
> > 
> >  #endif /* #ifndef GCC_OPTINFO_H */
> > --
> > 1.8.5.3
> > 

Reply via email to