Re: [PATCH 2/5] dumpfile.c: eliminate special-casing of dump_file/alt_dump_file

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

On Tue, Jul 31, 2018 at 5:34 PM David Malcolm <dmalcolm@redhat.com> wrote:
>
> On Tue, 2018-07-31 at 14:53 +0200, Richard Biener wrote:
> > On Fri, Jul 27, 2018 at 11:48 PM David Malcolm <dmalcolm@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???)

I think unless problems arise it should be a low-priority task.  I'd rather
see the new codes used throughout the codebase to simplify dump stuff.

Richard.

> > 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
> > >