v2: Formatted printing for dump_* in the middle-end

Message ID 1533232447-54802-1-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series
  • v2: Formatted printing for dump_* in the middle-end
Related show

Commit Message

David Malcolm Aug. 2, 2018, 5:54 p.m.
On Tue, 2018-07-31 at 19:56 +0000, Joseph Myers wrote:
> On Tue, 31 Jul 2018, David Malcolm wrote:
> 
> > I didn't exhaustively check every callsite to the changed calls;
> > I'm
> > assuming that -Wformat during bootstrap has effectively checked
> > that
> > for me.  Though now I think about it, I note that we use
> > HOST_WIDE_INT_PRINT_DEC in many places: is this guaranteed to be a
> > valid input to pp_format on all of our configurations?
> 
> HOST_WIDE_INT_PRINT_DEC should not be considered safe with pp_format 
> (although since r197049 may have effectively stopped using %I64 on
> MinGW 
> hosts, I'm not sure if there are current cases where it won't
> work).  
> Rather, it is the job of pp_format to map the 'w' length specifier
> to 
> HOST_WIDE_INT_PRINT_DEC etc.
> 
> I think it clearly makes for cleaner code to limit use of 
> HOST_WIDE_INT_PRINT_* to as few places as possible and to prefer use
> of 
> internal printf-like functions that accept formats such as %wd where 
> possible.

Thanks.

I grepped the tree for every use of HOST_WIDE_INT_PRINT* and found
all that were within dump_printf[_loc] calls.  All such uses were
within files of the form "gcc/tree-vect*.c".

Here's an updated version of the patch (v2) which fixes those
callsites to use "%wd" or "%wu"; the dumpfile.c changes are as
per v1.

Changed in v2:
* rebased to after r263239 (which also touched c-format.c/h)
* avoid HOST_WIDE_INT_PRINT* within dump_printf* calls (in
  gcc/tree-vect*.c)

I didn't add test coverage for %wd and %wu, as pretty-print.c already
has selftest coverage for these.

Richard's review of the v1 patch was:
"The patch is OK if C family maintainers agree on their parts."
so I'm looking for review of:
(a) the c-format.c changes (Joseph?), and
(b) the tree-vect*.c changes (Richard?  Joseph?)

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

Is this patch OK for trunk?

Thanks
Dave


Blurb from v1, for reference:

This patch converts dump_print and dump_printf_loc from using
printf (and thus ATTRIBUTE_PRINTF) to using a new pretty-printer
based on pp_format, which supports formatting middle-end types.

In particular, the following codes are implemented (in addition
to the standard pretty_printer ones):

   %E: gimple *:
       Equivalent to: dump_gimple_expr (MSG_*, TDF_SLIM, stmt, 0)
   %G: gimple *:
       Equivalent to: dump_gimple_stmt (MSG_*, TDF_SLIM, stmt, 0)
   %T: tree:
       Equivalent to: dump_generic_expr (MSG_*, arg, TDF_SLIM).

Hence it becomes possible to convert e.g.:

  if (dump_enabled_p ())
    {
      dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
                       "not vectorized: different sized vector "
                       "types in statement, ");
      dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, vectype);
      dump_printf (MSG_MISSED_OPTIMIZATION, " and ");
      dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, nunits_vectype);
      dump_printf (MSG_MISSED_OPTIMIZATION, "\n");
    }

into a one-liner:

  if (dump_enabled_p ())
    dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
                     "not vectorized: different sized vector "
                     "types in statement, %T and %T\n",
                     vectype, nunits_vectype);

Unlike regular pretty-printers, this one captures optinfo_item
instances for the formatted chunks as appropriate, so that when
written out to a JSON optimization record, the relevant parts of
the message are labelled by type, and by source location (so that
e.g. %G is entirely equivalent to using dump_gimple_stmt).

dump_printf and dump_printf_loc become marked with
ATTRIBUTE_GCC_DUMP_PRINTF, which the patch also implements.

gcc/c-family/ChangeLog:
	* c-format.c (enum format_type): Add gcc_dump_printf_format_type.
	(gcc_dump_printf_length_specs): New.
	(gcc_dump_printf_flag_pairs): New.
	(gcc_dump_printf_flag_specs): New.
	(gcc_dump_printf_char_table): New.
	(format_types_orig): Add entry for "gcc_dump_printf".
	(init_dynamic_diag_info): Set up length_char_specs and
	conversion_specs for gcc_dump_printf_format_type.
	(handle_format_attribute): Handle gcc_dump_printf_format_type.

gcc/ChangeLog:
	* dump-context.h: Include "dumpfile.h".
	(dump_context::dump_printf_va): Convert final param from va_list
	to va_list *.  Convert from ATTRIBUTE_PRINTF to
	ATTRIBUTE_GCC_DUMP_PRINTF.
	(dump_context::dump_printf_loc_va): Likewise.
	* dumpfile.c: Include "stringpool.h".
	(make_item_for_dump_printf_va): Delete.
	(make_item_for_dump_printf): Delete.
	(class dump_pretty_printer): New class.
	(dump_pretty_printer::dump_pretty_printer): New ctor.
	(dump_pretty_printer::emit_items): New member function.
	(dump_pretty_printer::emit_any_pending_textual_chunks): New member
	function.
	(dump_pretty_printer::emit_item): New member function.
	(dump_pretty_printer::stash_item): New member function.
	(dump_pretty_printer::format_decoder_cb): New member function.
	(dump_pretty_printer::decode_format): New member function.
	(dump_context::dump_printf_va): Reimplement in terms of
	dump_pretty_printer.
	(dump_context::dump_printf_loc_va): Convert final param from va_list
	to va_list *.
	(dump_context::begin_scope): Reimplement call to
	make_item_for_dump_printf.
	(dump_printf): Update for change to dump_printf_va.
	(dump_printf_loc): Likewise.
	(selftest::test_capture_of_dump_calls): Convert "stmt" from
	greturn * to gimple *.  Add a test_decl.  Add tests of dump_printf
	with %T, %E, and %G.
	* dumpfile.h (ATTRIBUTE_GCC_DUMP_PRINTF): New macro.
	(dump_printf): Replace ATTRIBUTE_PRINTF_2 with
	ATTRIBUTE_GCC_DUMP_PRINTF (2, 3).
	(dump_printf_loc): Replace ATTRIBUTE_PRINTF_3 with
	ATTRIBUTE_GCC_DUMP_PRINTF (3, 0).
	* tree-vect-data-refs.c (vect_lanes_optab_supported_p): Convert
	use of HOST_WIDE_INT_PRINT_DEC on unsigned HOST_WIDE_INT "count"
	within a dump_printf_loc call to "%wu".
	(vector_alignment_reachable_p): Merge two dump_printf[_loc] calls,
	converting a use of HOST_WIDE_INT_PRINT_DEC to "%wd".  Add a
	missing space after "=".
	* tree-vect-loop.c (vect_analyze_loop_2) Within a dump_printf
	call, convert use of HOST_WIDE_INT_PRINT_DEC to "%wd".
	* tree-vect-slp.c (vect_slp_bb): Within a dump_printf_loc call,
	convert use of HOST_WIDE_INT_PRINT_UNSIGNED to "%wu".
	* tree-vectorizer.c (try_vectorize_loop_1): Likewise.  Remove
	duplicate "vectorized" from message.

gcc/testsuite/ChangeLog:
	* gcc.dg/format/gcc_diag-1.c: Fix typo.  Add test coverage for
	gcc_dump_printf.
	* gcc.dg/format/gcc_diag-10.c: Add gimple typedef.  Add test
	coverage for gcc_dump_printf.
---
 gcc/c-family/c-format.c                   |  36 ++-
 gcc/dump-context.h                        |   7 +-
 gcc/dumpfile.c                            | 358 +++++++++++++++++++++++++++---
 gcc/dumpfile.h                            |  20 +-
 gcc/testsuite/gcc.dg/format/gcc_diag-1.c  |  15 +-
 gcc/testsuite/gcc.dg/format/gcc_diag-10.c |  26 +++
 gcc/tree-vect-data-refs.c                 |   8 +-
 gcc/tree-vect-loop.c                      |   2 +-
 gcc/tree-vect-slp.c                       |   3 +-
 gcc/tree-vectorizer.c                     |   4 +-
 10 files changed, 429 insertions(+), 50 deletions(-)

Comments

Joseph Myers Aug. 9, 2018, 10:11 p.m. | #1
On Thu, 2 Aug 2018, David Malcolm wrote:

> (a) the c-format.c changes (Joseph?), and

The c-format.c changes are OK.
Jeff Law Aug. 17, 2018, 4:08 a.m. | #2
On 08/02/2018 11:54 AM, David Malcolm wrote:
> On Tue, 2018-07-31 at 19:56 +0000, Joseph Myers wrote:
>> On Tue, 31 Jul 2018, David Malcolm wrote:
>>
>>> I didn't exhaustively check every callsite to the changed calls;
>>> I'm
>>> assuming that -Wformat during bootstrap has effectively checked
>>> that
>>> for me.  Though now I think about it, I note that we use
>>> HOST_WIDE_INT_PRINT_DEC in many places: is this guaranteed to be a
>>> valid input to pp_format on all of our configurations?
>> HOST_WIDE_INT_PRINT_DEC should not be considered safe with pp_format 
>> (although since r197049 may have effectively stopped using %I64 on
>> MinGW 
>> hosts, I'm not sure if there are current cases where it won't
>> work).  
>> Rather, it is the job of pp_format to map the 'w' length specifier
>> to 
>> HOST_WIDE_INT_PRINT_DEC etc.
>>
>> I think it clearly makes for cleaner code to limit use of 
>> HOST_WIDE_INT_PRINT_* to as few places as possible and to prefer use
>> of 
>> internal printf-like functions that accept formats such as %wd where 
>> possible.
> Thanks.
> 
> I grepped the tree for every use of HOST_WIDE_INT_PRINT* and found
> all that were within dump_printf[_loc] calls.  All such uses were
> within files of the form "gcc/tree-vect*.c".
> 
> Here's an updated version of the patch (v2) which fixes those
> callsites to use "%wd" or "%wu"; the dumpfile.c changes are as
> per v1.
> 
> Changed in v2:
> * rebased to after r263239 (which also touched c-format.c/h)
> * avoid HOST_WIDE_INT_PRINT* within dump_printf* calls (in
>   gcc/tree-vect*.c)
> 
> I didn't add test coverage for %wd and %wu, as pretty-print.c already
> has selftest coverage for these.
> 
> Richard's review of the v1 patch was:
> "The patch is OK if C family maintainers agree on their parts."
> so I'm looking for review of:
> (a) the c-format.c changes (Joseph?), and
> (b) the tree-vect*.c changes (Richard?  Joseph?)
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> 
> Is this patch OK for trunk?
> 
> Thanks
> Dave
> 
> 
> Blurb from v1, for reference:
> 
> This patch converts dump_print and dump_printf_loc from using
> printf (and thus ATTRIBUTE_PRINTF) to using a new pretty-printer
> based on pp_format, which supports formatting middle-end types.
> 
> In particular, the following codes are implemented (in addition
> to the standard pretty_printer ones):
> 
>    %E: gimple *:
>        Equivalent to: dump_gimple_expr (MSG_*, TDF_SLIM, stmt, 0)
>    %G: gimple *:
>        Equivalent to: dump_gimple_stmt (MSG_*, TDF_SLIM, stmt, 0)
>    %T: tree:
>        Equivalent to: dump_generic_expr (MSG_*, arg, TDF_SLIM).
> 
> Hence it becomes possible to convert e.g.:
> 
>   if (dump_enabled_p ())
>     {
>       dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>                        "not vectorized: different sized vector "
>                        "types in statement, ");
>       dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, vectype);
>       dump_printf (MSG_MISSED_OPTIMIZATION, " and ");
>       dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, nunits_vectype);
>       dump_printf (MSG_MISSED_OPTIMIZATION, "\n");
>     }
> 
> into a one-liner:
> 
>   if (dump_enabled_p ())
>     dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>                      "not vectorized: different sized vector "
>                      "types in statement, %T and %T\n",
>                      vectype, nunits_vectype);
> 
> Unlike regular pretty-printers, this one captures optinfo_item
> instances for the formatted chunks as appropriate, so that when
> written out to a JSON optimization record, the relevant parts of
> the message are labelled by type, and by source location (so that
> e.g. %G is entirely equivalent to using dump_gimple_stmt).
> 
> dump_printf and dump_printf_loc become marked with
> ATTRIBUTE_GCC_DUMP_PRINTF, which the patch also implements.
> 
> gcc/c-family/ChangeLog:
> 	* c-format.c (enum format_type): Add gcc_dump_printf_format_type.
> 	(gcc_dump_printf_length_specs): New.
> 	(gcc_dump_printf_flag_pairs): New.
> 	(gcc_dump_printf_flag_specs): New.
> 	(gcc_dump_printf_char_table): New.
> 	(format_types_orig): Add entry for "gcc_dump_printf".
> 	(init_dynamic_diag_info): Set up length_char_specs and
> 	conversion_specs for gcc_dump_printf_format_type.
> 	(handle_format_attribute): Handle gcc_dump_printf_format_type.
> 
> gcc/ChangeLog:
> 	* dump-context.h: Include "dumpfile.h".
> 	(dump_context::dump_printf_va): Convert final param from va_list
> 	to va_list *.  Convert from ATTRIBUTE_PRINTF to
> 	ATTRIBUTE_GCC_DUMP_PRINTF.
> 	(dump_context::dump_printf_loc_va): Likewise.
> 	* dumpfile.c: Include "stringpool.h".
> 	(make_item_for_dump_printf_va): Delete.
> 	(make_item_for_dump_printf): Delete.
> 	(class dump_pretty_printer): New class.
> 	(dump_pretty_printer::dump_pretty_printer): New ctor.
> 	(dump_pretty_printer::emit_items): New member function.
> 	(dump_pretty_printer::emit_any_pending_textual_chunks): New member
> 	function.
> 	(dump_pretty_printer::emit_item): New member function.
> 	(dump_pretty_printer::stash_item): New member function.
> 	(dump_pretty_printer::format_decoder_cb): New member function.
> 	(dump_pretty_printer::decode_format): New member function.
> 	(dump_context::dump_printf_va): Reimplement in terms of
> 	dump_pretty_printer.
> 	(dump_context::dump_printf_loc_va): Convert final param from va_list
> 	to va_list *.
> 	(dump_context::begin_scope): Reimplement call to
> 	make_item_for_dump_printf.
> 	(dump_printf): Update for change to dump_printf_va.
> 	(dump_printf_loc): Likewise.
> 	(selftest::test_capture_of_dump_calls): Convert "stmt" from
> 	greturn * to gimple *.  Add a test_decl.  Add tests of dump_printf
> 	with %T, %E, and %G.
> 	* dumpfile.h (ATTRIBUTE_GCC_DUMP_PRINTF): New macro.
> 	(dump_printf): Replace ATTRIBUTE_PRINTF_2 with
> 	ATTRIBUTE_GCC_DUMP_PRINTF (2, 3).
> 	(dump_printf_loc): Replace ATTRIBUTE_PRINTF_3 with
> 	ATTRIBUTE_GCC_DUMP_PRINTF (3, 0).
> 	* tree-vect-data-refs.c (vect_lanes_optab_supported_p): Convert
> 	use of HOST_WIDE_INT_PRINT_DEC on unsigned HOST_WIDE_INT "count"
> 	within a dump_printf_loc call to "%wu".
> 	(vector_alignment_reachable_p): Merge two dump_printf[_loc] calls,
> 	converting a use of HOST_WIDE_INT_PRINT_DEC to "%wd".  Add a
> 	missing space after "=".
> 	* tree-vect-loop.c (vect_analyze_loop_2) Within a dump_printf
> 	call, convert use of HOST_WIDE_INT_PRINT_DEC to "%wd".
> 	* tree-vect-slp.c (vect_slp_bb): Within a dump_printf_loc call,
> 	convert use of HOST_WIDE_INT_PRINT_UNSIGNED to "%wu".
> 	* tree-vectorizer.c (try_vectorize_loop_1): Likewise.  Remove
> 	duplicate "vectorized" from message.
> 
> gcc/testsuite/ChangeLog:
> 	* gcc.dg/format/gcc_diag-1.c: Fix typo.  Add test coverage for
> 	gcc_dump_printf.
> 	* gcc.dg/format/gcc_diag-10.c: Add gimple typedef.  Add test
> 	coverage for gcc_dump_printf.

> 
> diff --git a/gcc/dump-context.h b/gcc/dump-context.h
> index f40ea14..54095d3 100644
> --- a/gcc/dump-context.h
> +++ b/gcc/dump-context.h
> @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
>  #ifndef GCC_DUMP_CONTEXT_H
>  #define GCC_DUMP_CONTEXT_H 1
>  
> +#include "dumpfile.h" // for ATTRIBUTE_GCC_DUMP_PRINTF
No real need to document why you include something.  Just include what
you need.  If the need goes away, we do have tools which will help
identify unnecessary and duplicated #includes.

Per your comments above, I really only looked at the tree-vect* changes,
which are fine.  So I think you're covered now.  OK for the trunk.

jeff
David Malcolm Aug. 17, 2018, 6:24 p.m. | #3
On Thu, 2018-08-16 at 22:08 -0600, Jeff Law wrote:
> On 08/02/2018 11:54 AM, David Malcolm wrote:
> > On Tue, 2018-07-31 at 19:56 +0000, Joseph Myers wrote:
> > > On Tue, 31 Jul 2018, David Malcolm wrote:
> > > 
> > > > I didn't exhaustively check every callsite to the changed
> > > > calls;
> > > > I'm
> > > > assuming that -Wformat during bootstrap has effectively checked
> > > > that
> > > > for me.  Though now I think about it, I note that we use
> > > > HOST_WIDE_INT_PRINT_DEC in many places: is this guaranteed to
> > > > be a
> > > > valid input to pp_format on all of our configurations?
> > > 
> > > HOST_WIDE_INT_PRINT_DEC should not be considered safe with
> > > pp_format 
> > > (although since r197049 may have effectively stopped using %I64
> > > on
> > > MinGW 
> > > hosts, I'm not sure if there are current cases where it won't
> > > work).  
> > > Rather, it is the job of pp_format to map the 'w' length
> > > specifier
> > > to 
> > > HOST_WIDE_INT_PRINT_DEC etc.
> > > 
> > > I think it clearly makes for cleaner code to limit use of 
> > > HOST_WIDE_INT_PRINT_* to as few places as possible and to prefer
> > > use
> > > of 
> > > internal printf-like functions that accept formats such as %wd
> > > where 
> > > possible.
> > 
> > Thanks.
> > 
> > I grepped the tree for every use of HOST_WIDE_INT_PRINT* and found
> > all that were within dump_printf[_loc] calls.  All such uses were
> > within files of the form "gcc/tree-vect*.c".
> > 
> > Here's an updated version of the patch (v2) which fixes those
> > callsites to use "%wd" or "%wu"; the dumpfile.c changes are as
> > per v1.
> > 
> > Changed in v2:
> > * rebased to after r263239 (which also touched c-format.c/h)
> > * avoid HOST_WIDE_INT_PRINT* within dump_printf* calls (in
> >   gcc/tree-vect*.c)
> > 
> > I didn't add test coverage for %wd and %wu, as pretty-print.c
> > already
> > has selftest coverage for these.
> > 
> > Richard's review of the v1 patch was:
> > "The patch is OK if C family maintainers agree on their parts."
> > so I'm looking for review of:
> > (a) the c-format.c changes (Joseph?), and
> > (b) the tree-vect*.c changes (Richard?  Joseph?)
> > 
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> > 
> > Is this patch OK for trunk?
> > 
> > Thanks
> > Dave
> > 
> > 
> > Blurb from v1, for reference:
> > 
> > This patch converts dump_print and dump_printf_loc from using
> > printf (and thus ATTRIBUTE_PRINTF) to using a new pretty-printer
> > based on pp_format, which supports formatting middle-end types.
> > 
> > In particular, the following codes are implemented (in addition
> > to the standard pretty_printer ones):
> > 
> >    %E: gimple *:
> >        Equivalent to: dump_gimple_expr (MSG_*, TDF_SLIM, stmt, 0)
> >    %G: gimple *:
> >        Equivalent to: dump_gimple_stmt (MSG_*, TDF_SLIM, stmt, 0)
> >    %T: tree:
> >        Equivalent to: dump_generic_expr (MSG_*, arg, TDF_SLIM).
> > 
> > Hence it becomes possible to convert e.g.:
> > 
> >   if (dump_enabled_p ())
> >     {
> >       dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> >                        "not vectorized: different sized vector "
> >                        "types in statement, ");
> >       dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM,
> > vectype);
> >       dump_printf (MSG_MISSED_OPTIMIZATION, " and ");
> >       dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM,
> > nunits_vectype);
> >       dump_printf (MSG_MISSED_OPTIMIZATION, "\n");
> >     }
> > 
> > into a one-liner:
> > 
> >   if (dump_enabled_p ())
> >     dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> >                      "not vectorized: different sized vector "
> >                      "types in statement, %T and %T\n",
> >                      vectype, nunits_vectype);
> > 
> > Unlike regular pretty-printers, this one captures optinfo_item
> > instances for the formatted chunks as appropriate, so that when
> > written out to a JSON optimization record, the relevant parts of
> > the message are labelled by type, and by source location (so that
> > e.g. %G is entirely equivalent to using dump_gimple_stmt).
> > 
> > dump_printf and dump_printf_loc become marked with
> > ATTRIBUTE_GCC_DUMP_PRINTF, which the patch also implements.
> > 
> > gcc/c-family/ChangeLog:
> > 	* c-format.c (enum format_type): Add
> > gcc_dump_printf_format_type.
> > 	(gcc_dump_printf_length_specs): New.
> > 	(gcc_dump_printf_flag_pairs): New.
> > 	(gcc_dump_printf_flag_specs): New.
> > 	(gcc_dump_printf_char_table): New.
> > 	(format_types_orig): Add entry for "gcc_dump_printf".
> > 	(init_dynamic_diag_info): Set up length_char_specs and
> > 	conversion_specs for gcc_dump_printf_format_type.
> > 	(handle_format_attribute): Handle gcc_dump_printf_format_type.
> > 
> > gcc/ChangeLog:
> > 	* dump-context.h: Include "dumpfile.h".
> > 	(dump_context::dump_printf_va): Convert final param from
> > va_list
> > 	to va_list *.  Convert from ATTRIBUTE_PRINTF to
> > 	ATTRIBUTE_GCC_DUMP_PRINTF.
> > 	(dump_context::dump_printf_loc_va): Likewise.
> > 	* dumpfile.c: Include "stringpool.h".
> > 	(make_item_for_dump_printf_va): Delete.
> > 	(make_item_for_dump_printf): Delete.
> > 	(class dump_pretty_printer): New class.
> > 	(dump_pretty_printer::dump_pretty_printer): New ctor.
> > 	(dump_pretty_printer::emit_items): New member function.
> > 	(dump_pretty_printer::emit_any_pending_textual_chunks): New
> > member
> > 	function.
> > 	(dump_pretty_printer::emit_item): New member function.
> > 	(dump_pretty_printer::stash_item): New member function.
> > 	(dump_pretty_printer::format_decoder_cb): New member function.
> > 	(dump_pretty_printer::decode_format): New member function.
> > 	(dump_context::dump_printf_va): Reimplement in terms of
> > 	dump_pretty_printer.
> > 	(dump_context::dump_printf_loc_va): Convert final param from
> > va_list
> > 	to va_list *.
> > 	(dump_context::begin_scope): Reimplement call to
> > 	make_item_for_dump_printf.
> > 	(dump_printf): Update for change to dump_printf_va.
> > 	(dump_printf_loc): Likewise.
> > 	(selftest::test_capture_of_dump_calls): Convert "stmt" from
> > 	greturn * to gimple *.  Add a test_decl.  Add tests of
> > dump_printf
> > 	with %T, %E, and %G.
> > 	* dumpfile.h (ATTRIBUTE_GCC_DUMP_PRINTF): New macro.
> > 	(dump_printf): Replace ATTRIBUTE_PRINTF_2 with
> > 	ATTRIBUTE_GCC_DUMP_PRINTF (2, 3).
> > 	(dump_printf_loc): Replace ATTRIBUTE_PRINTF_3 with
> > 	ATTRIBUTE_GCC_DUMP_PRINTF (3, 0).
> > 	* tree-vect-data-refs.c (vect_lanes_optab_supported_p): Convert
> > 	use of HOST_WIDE_INT_PRINT_DEC on unsigned HOST_WIDE_INT
> > "count"
> > 	within a dump_printf_loc call to "%wu".
> > 	(vector_alignment_reachable_p): Merge two dump_printf[_loc]
> > calls,
> > 	converting a use of HOST_WIDE_INT_PRINT_DEC to "%wd".  Add a
> > 	missing space after "=".
> > 	* tree-vect-loop.c (vect_analyze_loop_2) Within a dump_printf
> > 	call, convert use of HOST_WIDE_INT_PRINT_DEC to "%wd".
> > 	* tree-vect-slp.c (vect_slp_bb): Within a dump_printf_loc call,
> > 	convert use of HOST_WIDE_INT_PRINT_UNSIGNED to "%wu".
> > 	* tree-vectorizer.c (try_vectorize_loop_1): Likewise.  Remove
> > 	duplicate "vectorized" from message.
> > 
> > gcc/testsuite/ChangeLog:
> > 	* gcc.dg/format/gcc_diag-1.c: Fix typo.  Add test coverage for
> > 	gcc_dump_printf.
> > 	* gcc.dg/format/gcc_diag-10.c: Add gimple typedef.  Add test
> > 	coverage for gcc_dump_printf.
> > 
> > diff --git a/gcc/dump-context.h b/gcc/dump-context.h
> > index f40ea14..54095d3 100644
> > --- a/gcc/dump-context.h
> > +++ b/gcc/dump-context.h
> > @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not
> > see
> >  #ifndef GCC_DUMP_CONTEXT_H
> >  #define GCC_DUMP_CONTEXT_H 1
> >  
> > +#include "dumpfile.h" // for ATTRIBUTE_GCC_DUMP_PRINTF
> 
> No real need to document why you include something.  Just include
> what
> you need.  If the need goes away, we do have tools which will help
> identify unnecessary and duplicated #includes.

I've removed the comment.

> Per your comments above, I really only looked at the tree-vect*
> changes,
> which are fine.  So I think you're covered now.  OK for the trunk.

Thanks.

This is now in trunk as r263626 (I rebased and retested it before
committing).

Dave
Jakub Jelinek Aug. 27, 2018, 6:57 a.m. | #4
On Thu, Aug 02, 2018 at 01:54:07PM -0400, David Malcolm wrote:
> +/* An attribute for annotating formatting printing functions that use
> +   the dumpfile/optinfo formatting codes.  These are the pretty_printer
> +   format codes (see pretty-print.c), with additional codes for middle-end
> +   specific entities (see dumpfile.c).  */
> +
> +#if GCC_VERSION >= 3005
> +#define ATTRIBUTE_GCC_DUMP_PRINTF(m, n) \
> +  __attribute__ ((__format__ (__gcc_dump_printf__, m ,n))) \
> +  ATTRIBUTE_NONNULL(m)
> +#else
> +#define ATTRIBUTE_GCC_DUMP_PRINTF(m, n) ATTRIBUTE_NONNULL(m)
> +#endif

Why >= 3005 rather than >= 9000 ?
GCC 8 and earlier will not handle that format attribute anyway and will just
loudly complain.

	Jakub

Patch

diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index dc0e756..d590805 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -44,6 +44,7 @@  enum format_type { printf_format_type, asm_fprintf_format_type,
 		   gcc_diag_format_type, gcc_tdiag_format_type,
 		   gcc_cdiag_format_type,
 		   gcc_cxxdiag_format_type, gcc_gfc_format_type,
+		   gcc_dump_printf_format_type,
 		   gcc_objc_string_format_type,
 		   format_type_error = -1};
 
@@ -461,6 +462,7 @@  static const format_length_info gcc_diag_length_specs[] =
 #define gcc_tdiag_length_specs gcc_diag_length_specs
 #define gcc_cdiag_length_specs gcc_diag_length_specs
 #define gcc_cxxdiag_length_specs gcc_diag_length_specs
+#define gcc_dump_printf_length_specs gcc_diag_length_specs
 
 /* This differs from printf_length_specs only in that "Z" is not accepted.  */
 static const format_length_info scanf_length_specs[] =
@@ -550,6 +552,7 @@  static const format_flag_pair gcc_diag_flag_pairs[] =
 #define gcc_cdiag_flag_pairs gcc_diag_flag_pairs
 #define gcc_cxxdiag_flag_pairs gcc_diag_flag_pairs
 #define gcc_gfc_flag_pairs gcc_diag_flag_pairs
+#define gcc_dump_printf_flag_pairs gcc_diag_flag_pairs
 
 static const format_flag_spec gcc_diag_flag_specs[] =
 {
@@ -565,6 +568,7 @@  static const format_flag_spec gcc_diag_flag_specs[] =
 #define gcc_cdiag_flag_specs gcc_diag_flag_specs
 #define gcc_cxxdiag_flag_specs gcc_diag_flag_specs
 #define gcc_gfc_flag_specs gcc_diag_flag_specs
+#define gcc_dump_printf_flag_specs gcc_diag_flag_specs
 
 static const format_flag_spec scanf_flag_specs[] =
 {
@@ -786,6 +790,22 @@  static const format_char_info gcc_gfc_char_table[] =
   { NULL,  0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
 };
 
+static const format_char_info gcc_dump_printf_char_table[] =
+{
+  /* The conversion specifiers implemented within pp_format.  */
+  PP_FORMAT_CHAR_TABLE,
+
+  /* Custom conversion specifiers implemented by dump_pretty_printer.  */
+
+  /* E and G require a "gimple *" argument at runtime.  */
+  { "EG",   1, STD_C89, { T89_G,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "", "\"",   NULL },
+
+  /* T requires a "tree" at runtime.  */
+  { "T",   1, STD_C89, { T89_T,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "", "\"",   NULL },
+
+  { NULL,  0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
+};
+
 static const format_char_info scan_char_table[] =
 {
   /* C89 conversion specifiers.  */
@@ -885,6 +905,13 @@  static const format_kind_info format_types_orig[] =
     0, 0, 0, 0, 0, 0,
     NULL, NULL
   },
+  { "gcc_dump_printf",   gcc_dump_printf_length_specs,
+    gcc_dump_printf_char_table, "q+#", NULL,
+    gcc_dump_printf_flag_specs, gcc_dump_printf_flag_pairs,
+    FMT_FLAG_ARG_CONVERT,
+    0, 0, 'p', 0, 'L', 0,
+    NULL, &integer_type_node
+  },
   { "NSString",   NULL,  NULL, NULL, NULL,
     NULL, NULL,
     FMT_FLAG_ARG_CONVERT|FMT_FLAG_PARSE_ARG_CONVERT_EXTERNAL, 0, 0, 0, 0, 0, 0,
@@ -3905,6 +3932,7 @@  init_dynamic_diag_info (void)
 	  dynamic_format_types[gcc_tdiag_format_type].length_char_specs =
 	  dynamic_format_types[gcc_cdiag_format_type].length_char_specs =
 	  dynamic_format_types[gcc_cxxdiag_format_type].length_char_specs =
+	  dynamic_format_types[gcc_dump_printf_format_type].length_char_specs =
 	  diag_ls = (format_length_info *)
 		    xmemdup (gcc_diag_length_specs,
 			     sizeof (gcc_diag_length_specs),
@@ -3931,6 +3959,8 @@  init_dynamic_diag_info (void)
     gcc_cdiag_char_table;
   dynamic_format_types[gcc_cxxdiag_format_type].conversion_specs =
     gcc_cxxdiag_char_table;
+  dynamic_format_types[gcc_dump_printf_format_type].conversion_specs =
+    gcc_dump_printf_char_table;
 }
 
 #ifdef TARGET_FORMAT_TYPES
@@ -4085,7 +4115,8 @@  handle_format_attribute (tree *node, tree ARG_UNUSED (name), tree args,
       || info.format_type == gcc_diag_format_type
       || info.format_type == gcc_tdiag_format_type
       || info.format_type == gcc_cdiag_format_type
-      || info.format_type == gcc_cxxdiag_format_type)
+      || info.format_type == gcc_cxxdiag_format_type
+      || info.format_type == gcc_dump_printf_format_type)
     {
       /* Our first time through, we have to make sure that our
 	 format_type data is allocated dynamically and is modifiable.  */
@@ -4107,7 +4138,8 @@  handle_format_attribute (tree *node, tree ARG_UNUSED (name), tree args,
       else if (info.format_type == gcc_diag_format_type
 	       || info.format_type == gcc_tdiag_format_type
 	       || info.format_type == gcc_cdiag_format_type
-	       || info.format_type == gcc_cxxdiag_format_type)
+	       || info.format_type == gcc_cxxdiag_format_type
+	       || info.format_type == gcc_dump_printf_format_type)
 	init_dynamic_diag_info ();
       else
 	gcc_unreachable ();
diff --git a/gcc/dump-context.h b/gcc/dump-context.h
index f40ea14..54095d3 100644
--- a/gcc/dump-context.h
+++ b/gcc/dump-context.h
@@ -22,6 +22,7 @@  along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_DUMP_CONTEXT_H
 #define GCC_DUMP_CONTEXT_H 1
 
+#include "dumpfile.h" // for ATTRIBUTE_GCC_DUMP_PRINTF
 #include "pretty-print.h"
 
 /* A class for handling the various dump_* calls.
@@ -73,11 +74,11 @@  class dump_context
 			      tree t);
 
   void dump_printf_va (dump_flags_t dump_kind, const char *format,
-		       va_list ap) ATTRIBUTE_PRINTF (3, 0);
+		       va_list *ap) ATTRIBUTE_GCC_DUMP_PRINTF (3, 0);
 
   void dump_printf_loc_va (dump_flags_t dump_kind, const dump_location_t &loc,
-			   const char *format, va_list ap)
-    ATTRIBUTE_PRINTF (4, 0);
+			   const char *format, va_list *ap)
+    ATTRIBUTE_GCC_DUMP_PRINTF (4, 0);
 
   template<unsigned int N, typename C>
   void dump_dec (dump_flags_t dump_kind, const poly_int<N, C> &value);
diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 76a2ee8..a81ab3e 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -38,6 +38,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "cgraph.h"
 #include "tree-pass.h" /* for "current_pass".  */
 #include "optinfo-emit-json.h"
+#include "stringpool.h" /* for get_identifier.  */
 
 /* If non-NULL, return one past-the-end of the matching SUBPART of
    the WHOLE string.  */
@@ -681,56 +682,262 @@  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.  */
+/* A subclass of pretty_printer for implementing dump_context::dump_printf_va.
+   In particular, the formatted chunks are captured as optinfo_item instances,
+   thus retaining metadata about the entities being dumped (e.g. source
+   locations), rather than just as plain text.  */
 
-static optinfo_item *
-make_item_for_dump_printf_va (const char *format, va_list ap)
-  ATTRIBUTE_PRINTF (1, 0);
+class dump_pretty_printer : public pretty_printer
+{
+public:
+  dump_pretty_printer (dump_context *context, dump_flags_t dump_kind);
 
-static optinfo_item *
-make_item_for_dump_printf_va (const char *format, va_list ap)
+  void emit_items (optinfo *dest);
+
+private:
+  /* Information on an optinfo_item that was generated during phase 2 of
+     formatting.  */
+  struct stashed_item
+  {
+    stashed_item (const char **buffer_ptr_, optinfo_item *item_)
+      : buffer_ptr (buffer_ptr_), item (item_) {}
+    const char **buffer_ptr;
+    optinfo_item *item;
+  };
+
+  static bool format_decoder_cb (pretty_printer *pp, text_info *text,
+				 const char *spec, int /*precision*/,
+				 bool /*wide*/, bool /*set_locus*/,
+				 bool /*verbose*/, bool */*quoted*/,
+				 const char **buffer_ptr);
+
+  bool decode_format (text_info *text, const char *spec,
+		      const char **buffer_ptr);
+
+  void stash_item (const char **buffer_ptr, optinfo_item *item);
+
+  void emit_any_pending_textual_chunks (optinfo *dest);
+
+  void emit_item (optinfo_item *item, optinfo *dest);
+
+  dump_context *m_context;
+  dump_flags_t m_dump_kind;
+  auto_vec<stashed_item> m_stashed_items;
+};
+
+/* dump_pretty_printer's ctor.  */
+
+dump_pretty_printer::dump_pretty_printer (dump_context *context,
+					  dump_flags_t dump_kind)
+: pretty_printer (), m_context (context), m_dump_kind (dump_kind),
+  m_stashed_items ()
+{
+  pp_format_decoder (this) = format_decoder_cb;
+}
+
+/* Phase 3 of formatting; compare with pp_output_formatted_text.
+
+   Emit optinfo_item instances for the various formatted chunks from phases
+   1 and 2 (i.e. pp_format).
+
+   Some chunks may already have had their items built (during decode_format).
+   These chunks have been stashed into m_stashed_items; we emit them here.
+
+   For all other purely textual chunks, they are printed into
+   buffer->formatted_obstack, and then emitted as a textual optinfo_item.
+   This consolidates multiple adjacent text chunks into a single text
+   optinfo_item.  */
+
+void
+dump_pretty_printer::emit_items (optinfo *dest)
+{
+  output_buffer *buffer = pp_buffer (this);
+  struct chunk_info *chunk_array = buffer->cur_chunk_array;
+  const char **args = chunk_array->args;
+
+  gcc_assert (buffer->obstack == &buffer->formatted_obstack);
+  gcc_assert (buffer->line_length == 0);
+
+  unsigned stashed_item_idx = 0;
+  for (unsigned chunk = 0; args[chunk]; chunk++)
+    {
+      if (stashed_item_idx < m_stashed_items.length ()
+	  && args[chunk] == *m_stashed_items[stashed_item_idx].buffer_ptr)
+	{
+	  emit_any_pending_textual_chunks (dest);
+	  /* This chunk has a stashed item: use it.  */
+	  emit_item (m_stashed_items[stashed_item_idx++].item, dest);
+	}
+      else
+	/* This chunk is purely textual.  Print it (to
+	   buffer->formatted_obstack), so that we can consolidate adjacent
+	   chunks into one textual optinfo_item.  */
+	pp_string (this, args[chunk]);
+    }
+
+  emit_any_pending_textual_chunks (dest);
+
+  /* Ensure that we consumed all of stashed_items.  */
+  gcc_assert (stashed_item_idx == m_stashed_items.length ());
+
+  /* Deallocate the chunk structure and everything after it (i.e. the
+     associated series of formatted strings).  */
+  buffer->cur_chunk_array = chunk_array->prev;
+  obstack_free (&buffer->chunk_obstack, chunk_array);
+}
+
+/* Subroutine of dump_pretty_printer::emit_items
+   for consolidating multiple adjacent pure-text chunks into single
+   optinfo_items (in phase 3).  */
+
+void
+dump_pretty_printer::emit_any_pending_textual_chunks (optinfo *dest)
 {
-  char *formatted_text = xvasprintf (format, ap);
+  gcc_assert (buffer->obstack == &buffer->formatted_obstack);
+
+  /* Don't emit an item if the pending text is empty.  */
+  if (output_buffer_last_position_in_text (buffer) == NULL)
+    return;
+
+  char *formatted_text = xstrdup (pp_formatted_text (this));
   optinfo_item *item
     = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
 			formatted_text);
-  return item;
+  emit_item (item, dest);
+
+  /* Clear the pending text by unwinding formatted_text back to the start
+     of the buffer (without deallocating).  */
+  obstack_free (&buffer->formatted_obstack,
+		buffer->formatted_obstack.object_base);
 }
 
-/* Make an item for the given dump call.  */
+/* Emit ITEM and take ownership of it.  If DEST is non-NULL, add ITEM
+   to DEST; otherwise delete ITEM.  */
 
-static optinfo_item *
-make_item_for_dump_printf (const char *format, ...)
-  ATTRIBUTE_PRINTF (1, 2);
+void
+dump_pretty_printer::emit_item (optinfo_item *item, optinfo *dest)
+{
+  m_context->emit_item (item, m_dump_kind);
+  if (dest)
+    dest->add_item (item);
+  else
+    delete item;
+}
 
-static optinfo_item *
-make_item_for_dump_printf (const char *format, ...)
+/* Record that ITEM (generated in phase 2 of formatting) is to be used for
+   the chunk at BUFFER_PTR in phase 3 (by emit_items).  */
+
+void
+dump_pretty_printer::stash_item (const char **buffer_ptr, optinfo_item *item)
 {
-  va_list ap;
-  va_start (ap, format);
-  optinfo_item *item
-    = make_item_for_dump_printf_va (format, ap);
-  va_end (ap);
-  return item;
+  gcc_assert (buffer_ptr);
+  gcc_assert (item);
+
+  m_stashed_items.safe_push (stashed_item (buffer_ptr, item));
+}
+
+/* pp_format_decoder callback for dump_pretty_printer, and thus for
+   dump_printf and dump_printf_loc.
+
+   A wrapper around decode_format, for type-safety.  */
+
+bool
+dump_pretty_printer::format_decoder_cb (pretty_printer *pp, text_info *text,
+					const char *spec, int /*precision*/,
+					bool /*wide*/, bool /*set_locus*/,
+					bool /*verbose*/, bool */*quoted*/,
+					const char **buffer_ptr)
+{
+  dump_pretty_printer *opp = static_cast <dump_pretty_printer *> (pp);
+  return opp->decode_format (text, spec, buffer_ptr);
+}
+
+/* Format decoder for dump_pretty_printer, and thus for dump_printf and
+   dump_printf_loc.
+
+   Supported format codes (in addition to the standard pretty_printer ones)
+   are:
+
+   %E: gimple *:
+       Equivalent to: dump_gimple_expr (MSG_*, TDF_SLIM, stmt, 0)
+   %G: gimple *:
+       Equivalent to: dump_gimple_stmt (MSG_*, TDF_SLIM, stmt, 0)
+   %T: tree:
+       Equivalent to: dump_generic_expr (MSG_*, arg, TDF_SLIM).
+
+   FIXME: add symtab_node?
+
+   These format codes build optinfo_item instances, thus capturing metadata
+   about the arguments being dumped, as well as the textual output.  */
+
+bool
+dump_pretty_printer::decode_format (text_info *text, const char *spec,
+				       const char **buffer_ptr)
+{
+  /* Various format codes that imply making an optinfo_item and stashed it
+     for later use (to capture metadata, rather than plain text).  */
+  switch (*spec)
+    {
+    case 'E':
+      {
+	gimple *stmt = va_arg (*text->args_ptr, gimple *);
+
+	/* Make an item for the stmt, and stash it.  */
+	optinfo_item *item = make_item_for_dump_gimple_expr (stmt, 0, TDF_SLIM);
+	stash_item (buffer_ptr, item);
+	return true;
+      }
+
+    case 'G':
+      {
+	gimple *stmt = va_arg (*text->args_ptr, gimple *);
+
+	/* Make an item for the stmt, and stash it.  */
+	optinfo_item *item = make_item_for_dump_gimple_stmt (stmt, 0, TDF_SLIM);
+	stash_item (buffer_ptr, item);
+	return true;
+      }
+
+    case 'T':
+      {
+	tree t = va_arg (*text->args_ptr, tree);
+
+	/* Make an item for the tree, and stash it.  */
+	optinfo_item *item = make_item_for_dump_generic_expr (t, TDF_SLIM);
+	stash_item (buffer_ptr, item);
+	return true;
+      }
+
+    default:
+      return false;
+    }
 }
 
 /* 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)
+			      va_list *ap)
 {
-  optinfo_item *item = make_item_for_dump_printf_va (format, ap);
-  emit_item (item, dump_kind);
+  dump_pretty_printer pp (this, dump_kind);
 
+  text_info text;
+  text.err_no = errno;
+  text.args_ptr = ap;
+  text.format_spec = format;
+
+  /* Phases 1 and 2, using pp_format.  */
+  pp_format (&pp, &text);
+
+  /* Phase 3.  */
   if (optinfo_enabled_p ())
     {
       optinfo &info = ensure_pending_optinfo ();
       info.handle_dump_file_kind (dump_kind);
-      info.add_item (item);
+      pp.emit_items (&info);
     }
   else
-    delete item;
+    pp.emit_items (NULL);
 }
 
 /* Similar to dump_printf, except source location is also printed, and
@@ -739,7 +946,7 @@  dump_context::dump_printf_va (dump_flags_t dump_kind, const char *format,
 void
 dump_context::dump_printf_loc_va (dump_flags_t dump_kind,
 				  const dump_location_t &loc,
-				  const char *format, va_list ap)
+				  const char *format, va_list *ap)
 {
   dump_loc (dump_kind, loc);
   dump_printf_va (dump_kind, format, ap);
@@ -851,7 +1058,11 @@  dump_context::begin_scope (const char *name, const dump_location_t &loc)
   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);
+  pretty_printer pp;
+  pp_printf (&pp, "=== %s ===\n", name);
+  optinfo_item *item
+    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
+			xstrdup (pp_formatted_text (&pp)));
   emit_item (item, MSG_NOTE);
 
   if (optinfo_enabled_p ())
@@ -859,7 +1070,6 @@  dump_context::begin_scope (const char *name, const dump_location_t &loc)
       optinfo &info = begin_next_optinfo (loc);
       info.m_kind = OPTINFO_KIND_SCOPE;
       info.add_item (item);
-      end_any_optinfo ();
     }
   else
     delete item;
@@ -1006,7 +1216,7 @@  dump_printf (dump_flags_t dump_kind, const char *format, ...)
 {
   va_list ap;
   va_start (ap, format);
-  dump_context::get ().dump_printf_va (dump_kind, format, ap);
+  dump_context::get ().dump_printf_va (dump_kind, format, &ap);
   va_end (ap);
 }
 
@@ -1019,7 +1229,7 @@  dump_printf_loc (dump_flags_t dump_kind, const dump_location_t &loc,
 {
   va_list ap;
   va_start (ap, format);
-  dump_context::get ().dump_printf_loc_va (dump_kind, loc, format, ap);
+  dump_context::get ().dump_printf_loc_va (dump_kind, loc, format, &ap);
   va_end (ap);
 }
 
@@ -1808,9 +2018,12 @@  test_capture_of_dump_calls (const line_table_case &case_)
 
   dump_location_t loc = dump_location_t::from_location_t (where);
 
-  greturn *stmt = gimple_build_return (NULL);
+  gimple *stmt = gimple_build_return (NULL);
   gimple_set_location (stmt, where);
 
+  tree test_decl = build_decl (UNKNOWN_LOCATION, VAR_DECL,
+			       get_identifier ("test_decl"),
+			       integer_type_node);
   /* 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.  */
@@ -1834,6 +2047,89 @@  test_capture_of_dump_calls (const line_table_case &case_)
 	  }
       }
 
+      /* Test of dump_printf with %T.  */
+      {
+	temp_dump_context tmp (with_optinfo, MSG_ALL);
+	dump_printf (MSG_NOTE, "tree: %T", integer_zero_node);
+
+	ASSERT_DUMPED_TEXT_EQ (tmp, "tree: 0");
+	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 (), 2);
+	    ASSERT_IS_TEXT (info->get_item (0), "tree: ");
+	    ASSERT_IS_TREE (info->get_item (1), UNKNOWN_LOCATION, "0");
+	  }
+      }
+
+      /* Test of dump_printf with %E.  */
+      {
+	temp_dump_context tmp (with_optinfo, MSG_ALL);
+	dump_printf (MSG_NOTE, "gimple: %E", stmt);
+
+	ASSERT_DUMPED_TEXT_EQ (tmp, "gimple: return;");
+	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 (), 2);
+	    ASSERT_IS_TEXT (info->get_item (0), "gimple: ");
+	    ASSERT_IS_GIMPLE (info->get_item (1), where, "return;");
+	  }
+      }
+
+      /* Test of dump_printf with %G.  */
+      {
+	temp_dump_context tmp (with_optinfo, MSG_ALL);
+	dump_printf (MSG_NOTE, "gimple: %G", stmt);
+
+	ASSERT_DUMPED_TEXT_EQ (tmp, "gimple: return;\n");
+	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 (), 2);
+	    ASSERT_IS_TEXT (info->get_item (0), "gimple: ");
+	    ASSERT_IS_GIMPLE (info->get_item (1), where, "return;\n");
+	  }
+      }
+
+      /* dump_print_loc with multiple format codes.  This tests various
+	 things:
+	 - intermingling of text, format codes handled by the base
+	 pretty_printer, and dump-specific format codes
+	 - multiple dump-specific format codes: some consecutive, others
+	 separated by text, trailing text after the final one.  */
+      {
+	temp_dump_context tmp (with_optinfo, MSG_ALL);
+	dump_printf_loc (MSG_NOTE, loc, "before %T and %T"
+			 " %i consecutive %E%E after\n",
+			 integer_zero_node, test_decl, 42, stmt, stmt);
+
+	ASSERT_DUMPED_TEXT_EQ (tmp,
+			       "test.txt:5:10: note: before 0 and test_decl"
+			       " 42 consecutive return;return; after\n");
+	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 (), 8);
+	    ASSERT_IS_TEXT (info->get_item (0), "before ");
+	    ASSERT_IS_TREE (info->get_item (1), UNKNOWN_LOCATION, "0");
+	    ASSERT_IS_TEXT (info->get_item (2), " and ");
+	    ASSERT_IS_TREE (info->get_item (3), UNKNOWN_LOCATION, "test_decl");
+	    ASSERT_IS_TEXT (info->get_item (4), " 42 consecutive ");
+	    ASSERT_IS_GIMPLE (info->get_item (5), where, "return;");
+	    ASSERT_IS_GIMPLE (info->get_item (6), where, "return;");
+	    ASSERT_IS_TEXT (info->get_item (7), " after\n");
+	  }
+      }
+
       /* Tree, via dump_generic_expr.  */
       {
 	temp_dump_context tmp (with_optinfo, MSG_ALL);
diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
index 8de001d..0305d36 100644
--- a/gcc/dumpfile.h
+++ b/gcc/dumpfile.h
@@ -23,6 +23,19 @@  along with GCC; see the file COPYING3.  If not see
 
 #include "profile-count.h"
 
+/* An attribute for annotating formatting printing functions that use
+   the dumpfile/optinfo formatting codes.  These are the pretty_printer
+   format codes (see pretty-print.c), with additional codes for middle-end
+   specific entities (see dumpfile.c).  */
+
+#if GCC_VERSION >= 3005
+#define ATTRIBUTE_GCC_DUMP_PRINTF(m, n) \
+  __attribute__ ((__format__ (__gcc_dump_printf__, m ,n))) \
+  ATTRIBUTE_NONNULL(m)
+#else
+#define ATTRIBUTE_GCC_DUMP_PRINTF(m, n) ATTRIBUTE_NONNULL(m)
+#endif
+
 /* Different tree dump places.  When you add new tree dump places,
    extend the DUMP_FILES array in dumpfile.c.  */
 enum tree_dump_index
@@ -476,9 +489,12 @@  dump_enabled_p (void)
    to minimize the work done for the common case where dumps
    are disabled.  */
 
-extern void dump_printf (dump_flags_t, const char *, ...) ATTRIBUTE_PRINTF_2;
+extern void dump_printf (dump_flags_t, const char *, ...)
+  ATTRIBUTE_GCC_DUMP_PRINTF (2, 3);
+
 extern void dump_printf_loc (dump_flags_t, const dump_location_t &,
-			     const char *, ...) ATTRIBUTE_PRINTF_3;
+			     const char *, ...)
+  ATTRIBUTE_GCC_DUMP_PRINTF (3, 0);
 extern void dump_function (int phase, tree fn);
 extern void dump_basic_block (dump_flags_t, basic_block, int);
 extern void dump_generic_expr_loc (dump_flags_t, const dump_location_t &,
diff --git a/gcc/testsuite/gcc.dg/format/gcc_diag-1.c b/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
index 034e097..8761456 100644
--- a/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
+++ b/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
@@ -1,4 +1,4 @@ 
-/* Test for GCC diagnositc formats.  */
+/* Test for GCC diagnostic formats.  */
 /* Origin: Kaveh Ghazi <ghazi@caip.rutgers.edu> */
 /* { dg-do compile } */
 /* { dg-options "-Wformat" } */
@@ -24,6 +24,7 @@  extern int diag (const char *, ...) ATTRIBUTE_DIAG(__gcc_diag__);
 extern int tdiag (const char *, ...) ATTRIBUTE_DIAG(__gcc_tdiag__);
 extern int cdiag (const char *, ...) ATTRIBUTE_DIAG(__gcc_cdiag__);
 extern int cxxdiag (const char *, ...) ATTRIBUTE_DIAG(__gcc_cxxdiag__);
+extern int dump (const char *, ...) ATTRIBUTE_DIAG(__gcc_dump_printf__);
 
 void
 foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p,
@@ -39,36 +40,44 @@  foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p,
   tdiag ("%%");
   cdiag ("%%");
   cxxdiag ("%%");
+  dump ("%%");
   diag ("%d%i%o%u%x%c%s%p%%", i, i, u, u, u, i, s, p);
   tdiag ("%d%i%o%u%x%c%s%p%%", i, i, u, u, u, i, s, p);
   cdiag ("%d%i%o%u%x%c%s%p%%", i, i, u, u, u, i, s, p);
   cxxdiag ("%d%i%o%u%x%c%s%p%%", i, i, u, u, u, i, s, p);
+  dump ("%d%i%o%u%x%c%s%p%%", i, i, u, u, u, i, s, p);
   diag ("%qd%qi%qo%qu%qx%qc%qs%qp%<%%%'%>", i, i, u, u, u, i, s, p);
   tdiag ("%qd%qi%qo%qu%qx%qc%qs%qp%<%%%'%>", i, i, u, u, u, i, s, p);
   cdiag ("%qd%qi%qo%qu%qx%qc%qs%qp%<%%%'%>", i, i, u, u, u, i, s, p);
   cxxdiag ("%qd%qi%qo%qu%qx%qc%qs%qp%<%%%'%>", i, i, u, u, u, i, s, p);
+  dump ("%qd%qi%qo%qu%qx%qc%qs%qp%<%%%'%>", i, i, u, u, u, i, s, p);
   diag ("%ld%li%lo%lu%lx", l, l, ul, ul, ul);
   tdiag ("%ld%li%lo%lu%lx", l, l, ul, ul, ul);
   cdiag ("%ld%li%lo%lu%lx", l, l, ul, ul, ul);
   cxxdiag ("%ld%li%lo%lu%lx", l, l, ul, ul, ul);
+  dump ("%ld%li%lo%lu%lx", l, l, ul, ul, ul);
   diag ("%lld%lli%llo%llu%llx", ll, ll, ull, ull, ull);
   tdiag ("%lld%lli%llo%llu%llx", ll, ll, ull, ull, ull);
   cdiag ("%lld%lli%llo%llu%llx", ll, ll, ull, ull, ull);
   cxxdiag ("%lld%lli%llo%llu%llx", ll, ll, ull, ull, ull);
+  dump ("%lld%lli%llo%llu%llx", ll, ll, ull, ull, ull);
   diag ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull);
   tdiag ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull);
   cdiag ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull);
   cxxdiag ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull);
+  dump ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull);
   diag ("%.*s", i, s);
   tdiag ("%.*s", i, s);
   cdiag ("%.*s", i, s);
   cxxdiag ("%.*s", i, s);
+  dump ("%.*s", i, s);
 
   /* Extensions provided in the diagnostic framework.  */
   diag ("%m");
   tdiag ("%m");
   cdiag ("%m");
   cxxdiag ("%m");
+  dump ("%m");
 
   /* Quote directives to avoid "warning: conversion used unquoted." */
   tdiag ("%<%D%F%T%V%>", t1, t1, t1, t1);
@@ -94,20 +103,24 @@  foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p,
   tdiag ("%Z", v, v_len);
   cdiag ("%Z", v, v_len);
   cxxdiag ("%Z", v, v_len);
+  dump ("%Z", v, v_len);
 
   /* Bad stuff with extensions.  */
   diag ("%m", i); /* { dg-warning "format" "extra arg" } */
   tdiag ("%m", i); /* { dg-warning "format" "extra arg" } */
   cdiag ("%m", i); /* { dg-warning "format" "extra arg" } */
   cxxdiag ("%m", i); /* { dg-warning "format" "extra arg" } */
+  dump ("%m", i); /* { dg-warning "format" "extra arg" } */
   diag ("%#m"); /* { dg-warning "format" "bogus modifier" } */
   tdiag ("%#m"); /* { dg-warning "format" "bogus modifier" } */
   cdiag ("%#m"); /* { dg-warning "format" "bogus modifier" } */
   cxxdiag ("%#m"); /* { dg-warning "format" "bogus modifier" } */
+  dump ("%#m"); /* { dg-warning "format" "bogus modifier" } */
   diag ("%+m"); /* { dg-warning "format" "bogus modifier" } */
   tdiag ("%+m"); /* { dg-warning "format" "bogus modifier" } */
   cdiag ("%+m"); /* { dg-warning "format" "bogus modifier" } */
   cxxdiag ("%+m"); /* { dg-warning "format" "bogus modifier" } */
+  dump ("%+m"); /* { dg-warning "format" "bogus modifier" } */
   diag ("%D", t1); /* { dg-warning "format" "bogus tree" } */
   tdiag ("%A", t1); /* { dg-warning "format" "bogus tree" } */
   tdiag ("%E", t1);
diff --git a/gcc/testsuite/gcc.dg/format/gcc_diag-10.c b/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
index 2965509..2f6a002 100644
--- a/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
+++ b/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
@@ -19,12 +19,16 @@  typedef union tree_node *tree;
    the C test to find the symbol.  */
 typedef struct gimple gimple;
 
+/* Likewise for gimple.  */
+typedef struct gimple gimple;
+
 #define FORMAT(kind) __attribute__ ((format (__gcc_## kind ##__, 1, 2)))
 
 void diag (const char*, ...) FORMAT (diag);
 void cdiag (const char*, ...) FORMAT (cdiag);
 void tdiag (const char*, ...) FORMAT (tdiag);
 void cxxdiag (const char*, ...) FORMAT (cxxdiag);
+void dump (const char*, ...) FORMAT (dump_printf);
 
 void test_diag (tree t, gimple *gc)
 {
@@ -157,3 +161,25 @@  void test_cxxdiag (tree t, gimple *gc)
   cxxdiag ("%<%V%>", t);
   cxxdiag ("%<%X%>", t);
 }
+
+void test_dump (tree t, gimple *stmt)
+{
+  dump ("%<");   /* { dg-warning "unterminated quoting directive" } */
+  dump ("%>");   /* { dg-warning "unmatched quoting directive " } */
+  dump ("%<foo%<bar%>%>");   /* { dg-warning "nested quoting directive" } */
+
+  dump ("%R");       /* { dg-warning "unmatched color reset directive" } */
+  dump ("%r", "");   /* { dg-warning "unterminated color directive" } */
+  dump ("%r%r", "", "");   /* { dg-warning "unterminated color directive" } */
+  dump ("%r%R", "");
+  dump ("%r%r%R", "", "");
+  dump ("%r%R%r%R", "", "");
+
+  dump ("%<%R%>");      /* { dg-warning "unmatched color reset directive" } */
+  dump ("%<%r%>", "");  /* { dg-warning "unterminated color directive" } */
+  dump ("%<%r%R%>", "");
+
+  dump ("%E", stmt);
+  dump ("%T", t);
+  dump ("%G", stmt);
+}
diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index e20b502..d70d207 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -74,8 +74,7 @@  vect_lanes_optab_supported_p (const char *name, convert_optab optab,
 	{
 	  if (dump_enabled_p ())
 	    dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
-			     "no array mode for %s["
-			     HOST_WIDE_INT_PRINT_DEC "]\n",
+			     "no array mode for %s[%wu]\n",
 			     GET_MODE_NAME (mode), count);
 	  return false;
 	}
@@ -1249,9 +1248,8 @@  vector_alignment_reachable_p (dr_vec_info *dr_info)
       if (dump_enabled_p ())
 	{
 	  dump_printf_loc (MSG_NOTE, vect_location,
-	                   "data size =" HOST_WIDE_INT_PRINT_DEC, elmsize);
-	  dump_printf (MSG_NOTE,
-	               ". misalignment = %d.\n", DR_MISALIGNMENT (dr_info));
+	                   "data size = %wd. misalignment = %d.\n", elmsize,
+			   DR_MISALIGNMENT (dr_info));
 	}
       if (DR_MISALIGNMENT (dr_info) % elmsize)
 	{
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 92c01a2..985f25e 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -1958,7 +1958,7 @@  start_over:
       dump_printf_loc (MSG_NOTE, vect_location,
 		       "vectorization_factor = ");
       dump_dec (MSG_NOTE, vectorization_factor);
-      dump_printf (MSG_NOTE, ", niters = " HOST_WIDE_INT_PRINT_DEC "\n",
+      dump_printf (MSG_NOTE, ", niters = %wd\n",
 		   LOOP_VINFO_INT_NITERS (loop_vinfo));
     }
 
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index d0f6da4..88ec230 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -3033,8 +3033,7 @@  vect_slp_bb (basic_block bb)
 	  unsigned HOST_WIDE_INT bytes;
 	  if (current_vector_size.is_constant (&bytes))
 	    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location,
-			     "basic block part vectorized using "
-			     HOST_WIDE_INT_PRINT_UNSIGNED " byte "
+			     "basic block part vectorized using %wu byte "
 			     "vectors\n", bytes);
 	  else
 	    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location,
diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index d58729b..db4fb76 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -935,9 +935,7 @@  try_vectorize_loop_1 (hash_table<simduid_to_vf> *&simduid_to_vf_htab,
   unsigned HOST_WIDE_INT bytes;
   if (current_vector_size.is_constant (&bytes))
     dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location,
-		     "loop vectorized vectorized using "
-		     HOST_WIDE_INT_PRINT_UNSIGNED " byte "
-		     "vectors\n", bytes);
+		     "loop vectorized using %wu byte vectors\n", bytes);
   else
     dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location,
 		     "loop vectorized using variable length vectors\n");