[committed] Introduce dump_location_t

Message ID 1530023889-45131-1-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series
  • [committed] Introduce dump_location_t
Related show

Commit Message

David Malcolm June 26, 2018, 2:38 p.m.
On Mon, 2018-06-25 at 15:34 +0200, Richard Biener wrote:
> On Wed, Jun 20, 2018 at 6:34 PM David Malcolm <dmalcolm@redhat.com>

> wrote:

> > 

> > Here's v3 of the patch (one big patch this time, rather than a

> > kit).

> > 

> > Like the v2 patch kit, this patch reuses the existing dump API,

> > rather than inventing its own.

> > 

> > Specifically, it uses the dump_* functions in dumpfile.h that don't

> > take a FILE *, the ones that implicitly write to dump_file and/or

> > alt_dump_file.  I needed a name for them, so I've taken to calling

> > them the "structured dump API" (better name ideas welcome).

> > 

> > v3 eliminates v2's optinfo_guard class, instead using "dump_*_loc"

> > calls as delimiters when consolidating "dump_*" calls.  There's a

> > new dump_context class which has responsibility for consolidating

> > them into optimization records.

> > 

> > The dump_*_loc calls now capture more than just a location_t: they

> > capture the profile_count and the location in GCC's own sources

> > where

> > the dump is being emitted from.

> > 

> > This works by introducing a new "dump_location_t" class as the

> > argument of those dump_*_loc calls.  The dump_location_t can

> > be constructed from a gimple * or from an rtx_insn *, so that

> > rather than writing:

> > 

> >   dump_printf_loc (MSG_NOTE, gimple_location (stmt),

> >                    "some message: %i", 42);

> > 

> > you can write:

> > 

> >   dump_printf_loc (MSG_NOTE, stmt,

> >                    "some message: %i", 42);

> > 

> > and the dump_location_t constructor will grab the location_t and

> > profile_count of stmt, and the location of the "dump_printf_loc"

> > callsite (and gracefully handle "stmt" being NULL).

> > 

> > Earlier versions of the patch captured the location of the

> > dump_*_loc call via preprocessor hacks, or didn't work properly;

> > this version of the patch works more cleanly: internally,

> > dump_location_t is split into two new classes:

> >   * dump_user_location_t: the location_t and profile_count within

> >     the *user's code*, and

> >   * dump_impl_location_t: the __builtin_FILE/LINE/FUNCTION within

> >     the *implementation* code (i.e. GCC or a plugin), captured

> >     "automagically" via default params

> > 

> > These classes are sometimes used elsewhere in the code.  For

> > example, "vect_location" becomes a dump_user_location_t

> > (location_t and profile_count), so that in e.g:

> > 

> >   vect_location = find_loop_location (loop);

> > 

> > it's capturing the location_t and profile_count, and then when

> > it's used here:

> > 

> >   dump_printf_loc (MSG_NOTE, vect_location, "foo");

> > 

> > the dump_location_t is constructed from the vect_location

> > plus the dump_impl_location_t at that callsite.

> > 

> > In contrast, loop-unroll.c's report_unroll's "locus" param

> > becomes a dump_location_t: we're interested in where it was

> > called from, not in the locations of the various dump_*_loc calls

> > within it.

> > 

> > Previous versions of the patch captured a gimple *, and needed

> > GTY markers; in this patch, the dump_user_location_t is now just a

> > location_t and a profile_count.

> > 

> > The v2 patch added an overload for dump_printf_loc so that you

> > could pass in either a location_t, or the new type; this version

> > of the patch eliminates that: they all now take dump_location_t.

> > 

> > Doing so required adding support for rtx_insn *, so that one can

> > write this kind of thing in RTL passes:

> > 

> >   dump_printf_loc (MSG_NOTE, insn, "foo");

> > 

> > One knock-on effect is that get_loop_location now returns a

> > dump_user_location_t rather than a location_t, so that it has

> > hotness information.

> > 

> > Richi: would you like me to split out this location-handling

> > code into a separate patch?  (It's kind of redundant without

> > adding the remarks and optimization records work, but if that's

> > easier I can do it)

> 

> I think that would be easier because it doesn't require the JSON

> stuff and so I'll happily approve it.

> 

> Thus - trying to review that bits (and sorry for the delay).

> 

> +  location_t srcloc = loc.get_location_t ();

> +

>    if (dump_file && (dump_kind & pflags))

>      {

> -      dump_loc (dump_kind, dump_file, loc);

> +      dump_loc (dump_kind, dump_file, srcloc);

>        print_gimple_stmt (dump_file, gs, spc, dump_flags |

> extra_dump_flags);

>      }

> 

>    if (alt_dump_file && (dump_kind & alt_flags))

>      {

> -      dump_loc (dump_kind, alt_dump_file, loc);

> +      dump_loc (dump_kind, alt_dump_file, srcloc);

>        print_gimple_stmt (alt_dump_file, gs, spc, dump_flags |

> extra_dump_flags);

>      }

> +

> +  if (optinfo_enabled_p ())

> +    {

> +      optinfo &info = begin_next_optinfo (loc);

> +      info.handle_dump_file_kind (dump_kind);

> +      info.add_stmt (gs, extra_dump_flags);

> +    }

> 

> seeing this in multiple places.  I seem to remember that

> dump_file / alt_dump_file was suposed to handle dumping

> into two locations - a dump file and optinfo (or stdout).  This looks

> like the optinfo "stream" is even more separate.  Could that

> obsolete the alt_dump_file stream?  I'd need to review existing stuff

> in more detail to answer but maybe you already know from recently

> digging into this.


Possibly.  I attempted this in v1 of the patch, but it was mixed in with
a bunch of other stuff.  I'll have another go at doing this.

> Oh, and all the if (optinfo_enable_p ()) stuff is for the followup

> then, right?


Yes.

> I like the boiler-plate changes to dump_* using stuff a lot, so the

> infrastructure to do that (the location wrapping) and these boiler-

> plate

> changes are pre-approved if split out.


Thanks.  I split out the location wrapping, and have committed it to
trunk (r262149).  It's not clear to me exactly what other parts you liked,
so I'm going to try to split out more of the non-JSON bits in the
hope that some parts are good enough as-is, and I'll post them for review
as followups.

For reference, here's what I've committed (I added some obvious changes
to doc/optinfo.texi).

> I think the *_REMARK stuff should get attention of the respective

> maintainers - not sure what the difference between NOTE and REMARK

> is ;)


Me neither :)  I think it might come down to "this is purely a debugging
message for a pass maintainer" (MSG_NOTE) vs "this is a high-level thing
that an advanced user want to see" (MSG_REMARK???).

One idea that occurred to me: are we overusing dump_flags_t?  It's a
mixture of TDF_* bitfields (used by our internal dumps) plus MSG_*
bitfields (used with -fopt-info).  IIRC the only TDF_ bitfield that's
ever used with the MSG_* bitfields is TDF_DETAILS.  Might it make sense
to split out the MSG_* bitfields into a different type (dump_level_t???)
to reinforce this split?  (and thus the dump_* entrypoints that don't take
a FILE * would take this new type).  I'm not sure if this is a good idea
or not.

> Thanks and sorry again for the repeated delays...

> Richard.


Thanks
Dave

[...snip...]

gcc/ChangeLog:
	* cfgloop.c (get_loop_location): Convert return type from
	location_t to dump_user_location_t, replacing INSN_LOCATION lookups
	by implicit construction from rtx_insn *, and using
	dump_user_location_t::from_function_decl for the fallback case.
	* cfgloop.h (get_loop_location): Convert return type from
	location_t to dump_user_location_t.
	* cgraphunit.c (walk_polymorphic_call_targets): Update call to
	dump_printf_loc to pass in a dump_location_t rather than a
	location_t, via the gimple stmt.
	* coverage.c (get_coverage_counts): Update calls to
	dump_printf_loc to pass in dump_location_t rather than a
	location_t.
	* doc/optinfo.texi (Dump types): Convert example of
	dump_printf_loc from taking "locus" to taking "insn".  Update
	description of the "_loc" calls to cover dump_location_t.
	* dumpfile.c: Include "backend.h", "gimple.h", "rtl.h", and
	"selftest.h".
	(dump_user_location_t::dump_user_location_t): New constructors,
	from gimple *stmt and rtx_insn *.
	(dump_user_location_t::from_function_decl): New function.
	(dump_loc): Make static.
	(dump_gimple_stmt_loc): Convert param "loc" from location_t to
	const dump_location_t &.
	(dump_generic_expr_loc): Delete.
	(dump_printf_loc): Convert param "loc" from location_t to
	const dump_location_t &.
	(selftest::test_impl_location): New function.
	(selftest::dumpfile_c_tests): New function.
	* dumpfile.h: Include "profile-count.h".
	(class dump_user_location_t): New class.
	(struct dump_impl_location_t): New struct.
	(class dump_location_t): New class.
	(dump_printf_loc): Convert 2nd param from source_location to
	const dump_location_t &.
	(dump_generic_expr_loc): Delete.
	(dump_gimple_stmt_loc): Convert 2nd param from source_location to
	const dump_location_t &.
	* gimple-fold.c (fold_gimple_assign): Update call to
	dump_printf_loc to pass in a dump_location_t rather than a
	location_t, via the gimple stmt.
	(gimple_fold_call): Likewise.
	* gimple-loop-interchange.cc
	(loop_cand::analyze_iloop_reduction_var): Update for change to
	check_reduction_path.
	(tree_loop_interchange::interchange): Update for change to
	find_loop_location.
	* graphite-isl-ast-to-gimple.c (scop_to_isl_ast): Update for
	change in return-type of find_loop_location.
	(graphite_regenerate_ast_isl): Likewise.
	* graphite-optimize-isl.c (optimize_isl): Likewise.
	* graphite.c (graphite_transform_loops): Likewise.
	* ipa-devirt.c (ipa_devirt): Update call to dump_printf_loc to
	pass in a dump_location_t rather than a location_t, via the
	gimple stmt.
	* ipa-prop.c (ipa_make_edge_direct_to_target): Likewise.
	* ipa.c (walk_polymorphic_call_targets): Likewise.
	* loop-unroll.c (report_unroll): Convert "locus" param from
	location_t to dump_location_t.
	(decide_unrolling): Update for change to get_loop_location's
	return type.
	* omp-grid.c (struct grid_prop): Convert field "target_loc" from
	location_t to dump_user_location_t.
	(grid_find_single_omp_among_assignments_1): Updates calls to
	dump_printf_loc to pass in a dump_location_t rather than a
	location_t, via the gimple stmt.
	(grid_parallel_clauses_gridifiable): Convert "tloc" from
	location_t to dump_location_t.  Updates calls to dump_printf_loc
	to pass in a dump_location_t rather than a location_t, via the
	gimple stmt.
	(grid_inner_loop_gridifiable_p): Likewise.
	(grid_dist_follows_simple_pattern): Likewise.
	(grid_gfor_follows_tiling_pattern): Likewise.
	(grid_target_follows_gridifiable_pattern): Likewise.
	(grid_attempt_target_gridification): Convert initialization
	of local "grid" from memset to zero-initialization; FIXME: does
	this require C++11?  Update call to dump_printf_loc to pass in a
	optinfo_location rather than a location_t, via the gimple stmt.
	* profile.c (read_profile_edge_counts): Updates call to
	dump_printf_loc to pass in a dump_location_t rather than a
	location_t
	(compute_branch_probabilities): Likewise.
	* selftest-run-tests.c (selftest::run_tests): Call
	dumpfile_c_tests.
	* selftest.h (dumpfile_c_tests): New decl.
	* tree-loop-distribution.c (pass_loop_distribution::execute):
	Update for change in return type of find_loop_location.
	* tree-parloops.c (parallelize_loops): Likewise.
	* tree-ssa-loop-ivcanon.c (try_unroll_loop_completely): Convert
	"locus" from location_t to dump_user_location_t.
	(canonicalize_loop_induction_variables): Likewise.
	* tree-ssa-loop-ivopts.c (tree_ssa_iv_optimize_loop): Update
	for change in return type of find_loop_location.
	* tree-ssa-loop-niter.c (number_of_iterations_exit): Update call
	to dump_printf_loc to pass in a dump_location_t rather than a
	location_t, via the stmt.
	* tree-ssa-sccvn.c (eliminate_dom_walker::before_dom_children):
	Likewise.
	* tree-vect-loop-manip.c (find_loop_location): Convert return
	type from source_location to dump_user_location_t.
	(vect_do_peeling): Update for above change.
	(vect_loop_versioning): Update for change in type of
	vect_location.
	* tree-vect-loop.c (check_reduction_path): Convert "loc" param
	from location_t to dump_user_location_t.
	(vect_estimate_min_profitable_iters): Update for change in type
	of vect_location.
	* tree-vect-slp.c (vect_print_slp_tree): Convert param "loc" from
	location_t to dump_location_t.
	(vect_slp_bb): Update for change in type of vect_location.
	* tree-vectorizer.c (vect_location): Convert from source_location
	to dump_user_location_t.
	(try_vectorize_loop_1): Update for change in vect_location's type.
	(vectorize_loops): Likewise.
	(increase_alignment): Likewise.
	* tree-vectorizer.h (vect_location): Convert from source_location
	to dump_user_location_t.
	(find_loop_location): Convert return type from source_location to
	dump_user_location_t.
	(check_reduction_path): Convert 1st param from location_t to
	dump_user_location_t.
	* value-prof.c (check_counter): Update call to dump_printf_loc to
	pass in a dump_user_location_t rather than a location_t; update
	call to error_at for change in type of "locus".
	(check_ic_target): Update call to dump_printf_loc to
	pass in a dump_user_location_t rather than a location_t, via the
	call_stmt.
---
 gcc/cfgloop.c                    |  12 +--
 gcc/cfgloop.h                    |   2 +-
 gcc/cgraphunit.c                 |   3 +-
 gcc/coverage.c                   |  22 ++++--
 gcc/doc/optinfo.texi             |  16 +++-
 gcc/dumpfile.c                   | 133 ++++++++++++++++++++++++++-------
 gcc/dumpfile.h                   | 156 +++++++++++++++++++++++++++++++++++++--
 gcc/gimple-fold.c                |   6 +-
 gcc/gimple-loop-interchange.cc   |   4 +-
 gcc/graphite-isl-ast-to-gimple.c |   4 +-
 gcc/graphite-optimize-isl.c      |   4 +-
 gcc/graphite.c                   |   2 +-
 gcc/ipa-devirt.c                 |   3 +-
 gcc/ipa-prop.c                   |  10 +--
 gcc/ipa.c                        |   9 +--
 gcc/loop-unroll.c                |   4 +-
 gcc/omp-grid.c                   |  47 ++++++------
 gcc/profile.c                    |  14 +++-
 gcc/selftest-run-tests.c         |   1 +
 gcc/selftest.h                   |   1 +
 gcc/tree-loop-distribution.c     |   2 +-
 gcc/tree-parloops.c              |   3 +-
 gcc/tree-ssa-loop-ivcanon.c      |   8 +-
 gcc/tree-ssa-loop-ivopts.c       |   2 +-
 gcc/tree-ssa-loop-niter.c        |   2 +-
 gcc/tree-ssa-sccvn.c             |   3 +-
 gcc/tree-vect-loop-manip.c       |  16 ++--
 gcc/tree-vect-loop.c             |   8 +-
 gcc/tree-vect-slp.c              |   5 +-
 gcc/tree-vectorizer.c            |  14 ++--
 gcc/tree-vectorizer.h            |   8 +-
 gcc/value-prof.c                 |  15 ++--
 32 files changed, 386 insertions(+), 153 deletions(-)

-- 
1.8.5.3

Comments

Richard Biener June 28, 2018, 11:29 a.m. | #1
On Tue, Jun 26, 2018 at 3:54 PM David Malcolm <dmalcolm@redhat.com> wrote:
>

> On Mon, 2018-06-25 at 15:34 +0200, Richard Biener wrote:

> > On Wed, Jun 20, 2018 at 6:34 PM David Malcolm <dmalcolm@redhat.com>

> > wrote:

> > >

> > > Here's v3 of the patch (one big patch this time, rather than a

> > > kit).

> > >

> > > Like the v2 patch kit, this patch reuses the existing dump API,

> > > rather than inventing its own.

> > >

> > > Specifically, it uses the dump_* functions in dumpfile.h that don't

> > > take a FILE *, the ones that implicitly write to dump_file and/or

> > > alt_dump_file.  I needed a name for them, so I've taken to calling

> > > them the "structured dump API" (better name ideas welcome).

> > >

> > > v3 eliminates v2's optinfo_guard class, instead using "dump_*_loc"

> > > calls as delimiters when consolidating "dump_*" calls.  There's a

> > > new dump_context class which has responsibility for consolidating

> > > them into optimization records.

> > >

> > > The dump_*_loc calls now capture more than just a location_t: they

> > > capture the profile_count and the location in GCC's own sources

> > > where

> > > the dump is being emitted from.

> > >

> > > This works by introducing a new "dump_location_t" class as the

> > > argument of those dump_*_loc calls.  The dump_location_t can

> > > be constructed from a gimple * or from an rtx_insn *, so that

> > > rather than writing:

> > >

> > >   dump_printf_loc (MSG_NOTE, gimple_location (stmt),

> > >                    "some message: %i", 42);

> > >

> > > you can write:

> > >

> > >   dump_printf_loc (MSG_NOTE, stmt,

> > >                    "some message: %i", 42);

> > >

> > > and the dump_location_t constructor will grab the location_t and

> > > profile_count of stmt, and the location of the "dump_printf_loc"

> > > callsite (and gracefully handle "stmt" being NULL).

> > >

> > > Earlier versions of the patch captured the location of the

> > > dump_*_loc call via preprocessor hacks, or didn't work properly;

> > > this version of the patch works more cleanly: internally,

> > > dump_location_t is split into two new classes:

> > >   * dump_user_location_t: the location_t and profile_count within

> > >     the *user's code*, and

> > >   * dump_impl_location_t: the __builtin_FILE/LINE/FUNCTION within

> > >     the *implementation* code (i.e. GCC or a plugin), captured

> > >     "automagically" via default params

> > >

> > > These classes are sometimes used elsewhere in the code.  For

> > > example, "vect_location" becomes a dump_user_location_t

> > > (location_t and profile_count), so that in e.g:

> > >

> > >   vect_location = find_loop_location (loop);

> > >

> > > it's capturing the location_t and profile_count, and then when

> > > it's used here:

> > >

> > >   dump_printf_loc (MSG_NOTE, vect_location, "foo");

> > >

> > > the dump_location_t is constructed from the vect_location

> > > plus the dump_impl_location_t at that callsite.

> > >

> > > In contrast, loop-unroll.c's report_unroll's "locus" param

> > > becomes a dump_location_t: we're interested in where it was

> > > called from, not in the locations of the various dump_*_loc calls

> > > within it.

> > >

> > > Previous versions of the patch captured a gimple *, and needed

> > > GTY markers; in this patch, the dump_user_location_t is now just a

> > > location_t and a profile_count.

> > >

> > > The v2 patch added an overload for dump_printf_loc so that you

> > > could pass in either a location_t, or the new type; this version

> > > of the patch eliminates that: they all now take dump_location_t.

> > >

> > > Doing so required adding support for rtx_insn *, so that one can

> > > write this kind of thing in RTL passes:

> > >

> > >   dump_printf_loc (MSG_NOTE, insn, "foo");

> > >

> > > One knock-on effect is that get_loop_location now returns a

> > > dump_user_location_t rather than a location_t, so that it has

> > > hotness information.

> > >

> > > Richi: would you like me to split out this location-handling

> > > code into a separate patch?  (It's kind of redundant without

> > > adding the remarks and optimization records work, but if that's

> > > easier I can do it)

> >

> > I think that would be easier because it doesn't require the JSON

> > stuff and so I'll happily approve it.

> >

> > Thus - trying to review that bits (and sorry for the delay).

> >

> > +  location_t srcloc = loc.get_location_t ();

> > +

> >    if (dump_file && (dump_kind & pflags))

> >      {

> > -      dump_loc (dump_kind, dump_file, loc);

> > +      dump_loc (dump_kind, dump_file, srcloc);

> >        print_gimple_stmt (dump_file, gs, spc, dump_flags |

> > extra_dump_flags);

> >      }

> >

> >    if (alt_dump_file && (dump_kind & alt_flags))

> >      {

> > -      dump_loc (dump_kind, alt_dump_file, loc);

> > +      dump_loc (dump_kind, alt_dump_file, srcloc);

> >        print_gimple_stmt (alt_dump_file, gs, spc, dump_flags |

> > extra_dump_flags);

> >      }

> > +

> > +  if (optinfo_enabled_p ())

> > +    {

> > +      optinfo &info = begin_next_optinfo (loc);

> > +      info.handle_dump_file_kind (dump_kind);

> > +      info.add_stmt (gs, extra_dump_flags);

> > +    }

> >

> > seeing this in multiple places.  I seem to remember that

> > dump_file / alt_dump_file was suposed to handle dumping

> > into two locations - a dump file and optinfo (or stdout).  This looks

> > like the optinfo "stream" is even more separate.  Could that

> > obsolete the alt_dump_file stream?  I'd need to review existing stuff

> > in more detail to answer but maybe you already know from recently

> > digging into this.

>

> Possibly.  I attempted this in v1 of the patch, but it was mixed in with

> a bunch of other stuff.  I'll have another go at doing this.

>

> > Oh, and all the if (optinfo_enable_p ()) stuff is for the followup

> > then, right?

>

> Yes.

>

> > I like the boiler-plate changes to dump_* using stuff a lot, so the

> > infrastructure to do that (the location wrapping) and these boiler-

> > plate

> > changes are pre-approved if split out.

>

> Thanks.  I split out the location wrapping, and have committed it to

> trunk (r262149).  It's not clear to me exactly what other parts you liked,

> so I'm going to try to split out more of the non-JSON bits in the

> hope that some parts are good enough as-is, and I'll post them for review

> as followups.

>

> For reference, here's what I've committed (I added some obvious changes

> to doc/optinfo.texi).

>

> > I think the *_REMARK stuff should get attention of the respective

> > maintainers - not sure what the difference between NOTE and REMARK

> > is ;)

>

> Me neither :)  I think it might come down to "this is purely a debugging

> message for a pass maintainer" (MSG_NOTE) vs "this is a high-level thing

> that an advanced user want to see" (MSG_REMARK???).

>

> One idea that occurred to me: are we overusing dump_flags_t?  It's a

> mixture of TDF_* bitfields (used by our internal dumps) plus MSG_*

> bitfields (used with -fopt-info).  IIRC the only TDF_ bitfield that's

> ever used with the MSG_* bitfields is TDF_DETAILS.  Might it make sense

> to split out the MSG_* bitfields into a different type (dump_level_t???)

> to reinforce this split?  (and thus the dump_* entrypoints that don't take

> a FILE * would take this new type).  I'm not sure if this is a good idea

> or not.


Making it even more complicated doesn't make it easier to use it
"correctly".  So I'd rather try to simplify it.  How passes use
TDF_DETAILS vs. non-details is already highly inconsistent.  This
is why I liked the original optinfo work because it somehow made
user-interesting vs. developer-interesting with the same API
(and to the same dump-file).  Just that MSG_NOTE is exposed to
users while I think it should be developer-only...

IMHO the TDF_ stuff should control things at the stmt/tree/BB level,
thus be IL specific flags while the MSG_ stuff should control
pass-specific dumping detail.

Richard.


> > Thanks and sorry again for the repeated delays...

> > Richard.

>

> Thanks

> Dave

>

> [...snip...]

>

> gcc/ChangeLog:

>         * cfgloop.c (get_loop_location): Convert return type from

>         location_t to dump_user_location_t, replacing INSN_LOCATION lookups

>         by implicit construction from rtx_insn *, and using

>         dump_user_location_t::from_function_decl for the fallback case.

>         * cfgloop.h (get_loop_location): Convert return type from

>         location_t to dump_user_location_t.

>         * cgraphunit.c (walk_polymorphic_call_targets): Update call to

>         dump_printf_loc to pass in a dump_location_t rather than a

>         location_t, via the gimple stmt.

>         * coverage.c (get_coverage_counts): Update calls to

>         dump_printf_loc to pass in dump_location_t rather than a

>         location_t.

>         * doc/optinfo.texi (Dump types): Convert example of

>         dump_printf_loc from taking "locus" to taking "insn".  Update

>         description of the "_loc" calls to cover dump_location_t.

>         * dumpfile.c: Include "backend.h", "gimple.h", "rtl.h", and

>         "selftest.h".

>         (dump_user_location_t::dump_user_location_t): New constructors,

>         from gimple *stmt and rtx_insn *.

>         (dump_user_location_t::from_function_decl): New function.

>         (dump_loc): Make static.

>         (dump_gimple_stmt_loc): Convert param "loc" from location_t to

>         const dump_location_t &.

>         (dump_generic_expr_loc): Delete.

>         (dump_printf_loc): Convert param "loc" from location_t to

>         const dump_location_t &.

>         (selftest::test_impl_location): New function.

>         (selftest::dumpfile_c_tests): New function.

>         * dumpfile.h: Include "profile-count.h".

>         (class dump_user_location_t): New class.

>         (struct dump_impl_location_t): New struct.

>         (class dump_location_t): New class.

>         (dump_printf_loc): Convert 2nd param from source_location to

>         const dump_location_t &.

>         (dump_generic_expr_loc): Delete.

>         (dump_gimple_stmt_loc): Convert 2nd param from source_location to

>         const dump_location_t &.

>         * gimple-fold.c (fold_gimple_assign): Update call to

>         dump_printf_loc to pass in a dump_location_t rather than a

>         location_t, via the gimple stmt.

>         (gimple_fold_call): Likewise.

>         * gimple-loop-interchange.cc

>         (loop_cand::analyze_iloop_reduction_var): Update for change to

>         check_reduction_path.

>         (tree_loop_interchange::interchange): Update for change to

>         find_loop_location.

>         * graphite-isl-ast-to-gimple.c (scop_to_isl_ast): Update for

>         change in return-type of find_loop_location.

>         (graphite_regenerate_ast_isl): Likewise.

>         * graphite-optimize-isl.c (optimize_isl): Likewise.

>         * graphite.c (graphite_transform_loops): Likewise.

>         * ipa-devirt.c (ipa_devirt): Update call to dump_printf_loc to

>         pass in a dump_location_t rather than a location_t, via the

>         gimple stmt.

>         * ipa-prop.c (ipa_make_edge_direct_to_target): Likewise.

>         * ipa.c (walk_polymorphic_call_targets): Likewise.

>         * loop-unroll.c (report_unroll): Convert "locus" param from

>         location_t to dump_location_t.

>         (decide_unrolling): Update for change to get_loop_location's

>         return type.

>         * omp-grid.c (struct grid_prop): Convert field "target_loc" from

>         location_t to dump_user_location_t.

>         (grid_find_single_omp_among_assignments_1): Updates calls to

>         dump_printf_loc to pass in a dump_location_t rather than a

>         location_t, via the gimple stmt.

>         (grid_parallel_clauses_gridifiable): Convert "tloc" from

>         location_t to dump_location_t.  Updates calls to dump_printf_loc

>         to pass in a dump_location_t rather than a location_t, via the

>         gimple stmt.

>         (grid_inner_loop_gridifiable_p): Likewise.

>         (grid_dist_follows_simple_pattern): Likewise.

>         (grid_gfor_follows_tiling_pattern): Likewise.

>         (grid_target_follows_gridifiable_pattern): Likewise.

>         (grid_attempt_target_gridification): Convert initialization

>         of local "grid" from memset to zero-initialization; FIXME: does

>         this require C++11?  Update call to dump_printf_loc to pass in a

>         optinfo_location rather than a location_t, via the gimple stmt.

>         * profile.c (read_profile_edge_counts): Updates call to

>         dump_printf_loc to pass in a dump_location_t rather than a

>         location_t

>         (compute_branch_probabilities): Likewise.

>         * selftest-run-tests.c (selftest::run_tests): Call

>         dumpfile_c_tests.

>         * selftest.h (dumpfile_c_tests): New decl.

>         * tree-loop-distribution.c (pass_loop_distribution::execute):

>         Update for change in return type of find_loop_location.

>         * tree-parloops.c (parallelize_loops): Likewise.

>         * tree-ssa-loop-ivcanon.c (try_unroll_loop_completely): Convert

>         "locus" from location_t to dump_user_location_t.

>         (canonicalize_loop_induction_variables): Likewise.

>         * tree-ssa-loop-ivopts.c (tree_ssa_iv_optimize_loop): Update

>         for change in return type of find_loop_location.

>         * tree-ssa-loop-niter.c (number_of_iterations_exit): Update call

>         to dump_printf_loc to pass in a dump_location_t rather than a

>         location_t, via the stmt.

>         * tree-ssa-sccvn.c (eliminate_dom_walker::before_dom_children):

>         Likewise.

>         * tree-vect-loop-manip.c (find_loop_location): Convert return

>         type from source_location to dump_user_location_t.

>         (vect_do_peeling): Update for above change.

>         (vect_loop_versioning): Update for change in type of

>         vect_location.

>         * tree-vect-loop.c (check_reduction_path): Convert "loc" param

>         from location_t to dump_user_location_t.

>         (vect_estimate_min_profitable_iters): Update for change in type

>         of vect_location.

>         * tree-vect-slp.c (vect_print_slp_tree): Convert param "loc" from

>         location_t to dump_location_t.

>         (vect_slp_bb): Update for change in type of vect_location.

>         * tree-vectorizer.c (vect_location): Convert from source_location

>         to dump_user_location_t.

>         (try_vectorize_loop_1): Update for change in vect_location's type.

>         (vectorize_loops): Likewise.

>         (increase_alignment): Likewise.

>         * tree-vectorizer.h (vect_location): Convert from source_location

>         to dump_user_location_t.

>         (find_loop_location): Convert return type from source_location to

>         dump_user_location_t.

>         (check_reduction_path): Convert 1st param from location_t to

>         dump_user_location_t.

>         * value-prof.c (check_counter): Update call to dump_printf_loc to

>         pass in a dump_user_location_t rather than a location_t; update

>         call to error_at for change in type of "locus".

>         (check_ic_target): Update call to dump_printf_loc to

>         pass in a dump_user_location_t rather than a location_t, via the

>         call_stmt.

> ---

>  gcc/cfgloop.c                    |  12 +--

>  gcc/cfgloop.h                    |   2 +-

>  gcc/cgraphunit.c                 |   3 +-

>  gcc/coverage.c                   |  22 ++++--

>  gcc/doc/optinfo.texi             |  16 +++-

>  gcc/dumpfile.c                   | 133 ++++++++++++++++++++++++++-------

>  gcc/dumpfile.h                   | 156 +++++++++++++++++++++++++++++++++++++--

>  gcc/gimple-fold.c                |   6 +-

>  gcc/gimple-loop-interchange.cc   |   4 +-

>  gcc/graphite-isl-ast-to-gimple.c |   4 +-

>  gcc/graphite-optimize-isl.c      |   4 +-

>  gcc/graphite.c                   |   2 +-

>  gcc/ipa-devirt.c                 |   3 +-

>  gcc/ipa-prop.c                   |  10 +--

>  gcc/ipa.c                        |   9 +--

>  gcc/loop-unroll.c                |   4 +-

>  gcc/omp-grid.c                   |  47 ++++++------

>  gcc/profile.c                    |  14 +++-

>  gcc/selftest-run-tests.c         |   1 +

>  gcc/selftest.h                   |   1 +

>  gcc/tree-loop-distribution.c     |   2 +-

>  gcc/tree-parloops.c              |   3 +-

>  gcc/tree-ssa-loop-ivcanon.c      |   8 +-

>  gcc/tree-ssa-loop-ivopts.c       |   2 +-

>  gcc/tree-ssa-loop-niter.c        |   2 +-

>  gcc/tree-ssa-sccvn.c             |   3 +-

>  gcc/tree-vect-loop-manip.c       |  16 ++--

>  gcc/tree-vect-loop.c             |   8 +-

>  gcc/tree-vect-slp.c              |   5 +-

>  gcc/tree-vectorizer.c            |  14 ++--

>  gcc/tree-vectorizer.h            |   8 +-

>  gcc/value-prof.c                 |  15 ++--

>  32 files changed, 386 insertions(+), 153 deletions(-)

>

> diff --git a/gcc/cfgloop.c b/gcc/cfgloop.c

> index 8af793c..e27cd39 100644

> --- a/gcc/cfgloop.c

> +++ b/gcc/cfgloop.c

> @@ -1800,7 +1800,7 @@ loop_exits_from_bb_p (struct loop *loop, basic_block bb)

>

>  /* Return location corresponding to the loop control condition if possible.  */

>

> -location_t

> +dump_user_location_t

>  get_loop_location (struct loop *loop)

>  {

>    rtx_insn *insn = NULL;

> @@ -1819,7 +1819,7 @@ get_loop_location (struct loop *loop)

>        FOR_BB_INSNS_REVERSE (desc->in_edge->src, insn)

>          {

>            if (INSN_P (insn) && INSN_HAS_LOCATION (insn))

> -            return INSN_LOCATION (insn);

> +            return insn;

>          }

>      }

>    /* If loop has a single exit, then the loop control branch

> @@ -1829,24 +1829,24 @@ get_loop_location (struct loop *loop)

>        FOR_BB_INSNS_REVERSE (exit->src, insn)

>          {

>            if (INSN_P (insn) && INSN_HAS_LOCATION (insn))

> -            return INSN_LOCATION (insn);

> +            return insn;

>          }

>      }

>    /* Next check the latch, to see if it is non-empty.  */

>    FOR_BB_INSNS_REVERSE (loop->latch, insn)

>      {

>        if (INSN_P (insn) && INSN_HAS_LOCATION (insn))

> -        return INSN_LOCATION (insn);

> +        return insn;

>      }

>    /* Finally, if none of the above identifies the loop control branch,

>       return the first location in the loop header.  */

>    FOR_BB_INSNS (loop->header, insn)

>      {

>        if (INSN_P (insn) && INSN_HAS_LOCATION (insn))

> -        return INSN_LOCATION (insn);

> +        return insn;

>      }

>    /* If all else fails, simply return the current function location.  */

> -  return DECL_SOURCE_LOCATION (current_function_decl);

> +  return dump_user_location_t::from_function_decl (current_function_decl);

>  }

>

>  /* Records that every statement in LOOP is executed I_BOUND times.

> diff --git a/gcc/cfgloop.h b/gcc/cfgloop.h

> index af9bfab..80a31c4 100644

> --- a/gcc/cfgloop.h

> +++ b/gcc/cfgloop.h

> @@ -357,7 +357,7 @@ extern bool loop_exit_edge_p (const struct loop *, const_edge);

>  extern bool loop_exits_to_bb_p (struct loop *, basic_block);

>  extern bool loop_exits_from_bb_p (struct loop *, basic_block);

>  extern void mark_loop_exit_edges (void);

> -extern location_t get_loop_location (struct loop *loop);

> +extern dump_user_location_t get_loop_location (struct loop *loop);

>

>  /* Loops & cfg manipulation.  */

>  extern basic_block *get_loop_body (const struct loop *);

> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c

> index 04b6919..7cfb8a0 100644

> --- a/gcc/cgraphunit.c

> +++ b/gcc/cgraphunit.c

> @@ -928,8 +928,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets,

>             }

>            if (dump_enabled_p ())

>              {

> -             location_t locus = gimple_location_safe (edge->call_stmt);

> -             dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, locus,

> +             dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, edge->call_stmt,

>                                "devirtualizing call in %s to %s\n",

>                                edge->caller->name (), target->name ());

>             }

> diff --git a/gcc/coverage.c b/gcc/coverage.c

> index 84fff13..350cc45 100644

> --- a/gcc/coverage.c

> +++ b/gcc/coverage.c

> @@ -342,12 +342,16 @@ get_coverage_counts (unsigned counter, unsigned expected,

>        static int warned = 0;

>

>        if (!warned++ && dump_enabled_p ())

> -       dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, input_location,

> -                         (flag_guess_branch_prob

> -                          ? "file %s not found, execution counts estimated\n"

> -                          : "file %s not found, execution counts assumed to "

> -                            "be zero\n"),

> -                         da_file_name);

> +       {

> +         dump_user_location_t loc

> +           = dump_user_location_t::from_location_t (input_location);

> +         dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

> +                          (flag_guess_branch_prob

> +                           ? "file %s not found, execution counts estimated\n"

> +                           : "file %s not found, execution counts assumed to "

> +                           "be zero\n"),

> +                          da_file_name);

> +       }

>        return NULL;

>      }

>    if (PARAM_VALUE (PARAM_PROFILE_FUNC_INTERNAL_ID))

> @@ -378,7 +382,9 @@ get_coverage_counts (unsigned counter, unsigned expected,

>                     "its profile data (counter %qs)", id, ctr_names[counter]);

>        if (warning_printed && dump_enabled_p ())

>         {

> -          dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, input_location,

> +         dump_user_location_t loc

> +           = dump_user_location_t::from_location_t (input_location);

> +          dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

>                             "use -Wno-error=coverage-mismatch to tolerate "

>                             "the mismatch but performance may drop if the "

>                             "function is hot\n");

> @@ -386,7 +392,7 @@ get_coverage_counts (unsigned counter, unsigned expected,

>           if (!seen_error ()

>               && !warned++)

>             {

> -             dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, input_location,

> +             dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

>                                 "coverage mismatch ignored\n");

>               dump_printf (MSG_OPTIMIZED_LOCATIONS,

>                             flag_guess_branch_prob

> diff --git a/gcc/doc/optinfo.texi b/gcc/doc/optinfo.texi

> index 8c28501..6202802 100644

> --- a/gcc/doc/optinfo.texi

> +++ b/gcc/doc/optinfo.texi

> @@ -168,7 +168,7 @@ when any of the following flags is enabled

>

>  @example

>  int report_flags = MSG_OPTIMIZED_LOCATIONS | TDF_RTL | TDF_DETAILS;

> -dump_printf_loc (report_flags, locus,

> +dump_printf_loc (report_flags, insn,

>                   "loop turned into non-loop; it never loops.\n");

>  @end example

>

> @@ -181,7 +181,19 @@ Output gimple statement.

>

>  Note that the above methods also have variants prefixed with

>  @code{_loc}, such as @code{dump_printf_loc}, which are similar except

> -they also output the source location information.

> +they also output the source location information.  The @code{_loc} variants

> +take a @code{const dump_location_t &}.  This class can be constructed from

> +a @code{gimple *} or from a @code{rtx_insn *}, and so callers can pass

> +a @code{gimple *} or a @code{rtx_insn *} as the @code{_loc} argument.

> +The @code{dump_location_t} constructor will extract the source location

> +from the statement or instruction, along with the profile count, and

> +the location in GCC's own source code (or the plugin) from which the dump

> +call was emitted.  Only the source location is currently used.

> +There is also a @code{dump_user_location_t} class, capturing the

> +source location and profile count, but not the dump emission location,

> +so that locations in the user's code can be passed around.  This

> +can also be constructed from a @code{gimple *} and from a @code{rtx_insn *},

> +and it too can be passed as the @code{_loc} argument.

>

>  @end ftable

>

> diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c

> index 2f11284..122e420 100644

> --- a/gcc/dumpfile.c

> +++ b/gcc/dumpfile.c

> @@ -29,6 +29,10 @@ along with GCC; see the file COPYING3.  If not see

>  #include "profile-count.h"

>  #include "tree-cfg.h"

>  #include "langhooks.h"

> +#include "backend.h" /* for gimple.h.  */

> +#include "gimple.h" /* for dump_user_location_t ctor.  */

> +#include "rtl.h" /* for dump_user_location_t ctor.  */

> +#include "selftest.h"

>

>  /* If non-NULL, return one past-the-end of the matching SUBPART of

>     the WHOLE string.  */

> @@ -358,9 +362,51 @@ dump_open_alternate_stream (struct dump_file_info *dfi)

>    return stream;

>  }

>

> +/* Construct a dump_user_location_t from STMT (using its location and

> +   hotness).  */

> +

> +dump_user_location_t::dump_user_location_t (gimple *stmt)

> +: m_count (), m_loc (UNKNOWN_LOCATION)

> +{

> +  if (stmt)

> +    {

> +      if (stmt->bb)

> +       m_count = stmt->bb->count;

> +      m_loc = gimple_location (stmt);

> +    }

> +}

> +

> +/* Construct a dump_user_location_t from an RTL instruction (using its

> +   location and hotness).  */

> +

> +dump_user_location_t::dump_user_location_t (rtx_insn *insn)

> +: m_count (), m_loc (UNKNOWN_LOCATION)

> +{

> +  if (insn)

> +    {

> +      basic_block bb = BLOCK_FOR_INSN (insn);

> +      if (bb)

> +       m_count = bb->count;

> +      m_loc = INSN_LOCATION (insn);

> +    }

> +}

> +

> +/* Construct from a function declaration.  This one requires spelling out

> +   to avoid accidentally constructing from other kinds of tree.  */

> +

> +dump_user_location_t

> +dump_user_location_t::from_function_decl (tree fndecl)

> +{

> +  gcc_assert (fndecl);

> +

> +  // FIXME: profile count for function?

> +  return dump_user_location_t (profile_count (),

> +                              DECL_SOURCE_LOCATION (fndecl));

> +}

> +

>  /* Print source location on DFILE if enabled.  */

>

> -void

> +static void

>  dump_loc (dump_flags_t dump_kind, FILE *dfile, source_location loc)

>  {

>    if (dump_kind)

> @@ -393,18 +439,19 @@ dump_gimple_stmt (dump_flags_t dump_kind, dump_flags_t extra_dump_flags,

>  /* Similar to dump_gimple_stmt, except additionally print source location.  */

>

>  void

> -dump_gimple_stmt_loc (dump_flags_t dump_kind, source_location loc,

> +dump_gimple_stmt_loc (dump_flags_t dump_kind, const dump_location_t &loc,

>                       dump_flags_t extra_dump_flags, gimple *gs, int spc)

>  {

> +  location_t srcloc = loc.get_location_t ();

>    if (dump_file && (dump_kind & pflags))

>      {

> -      dump_loc (dump_kind, dump_file, loc);

> +      dump_loc (dump_kind, dump_file, srcloc);

>        print_gimple_stmt (dump_file, gs, spc, dump_flags | extra_dump_flags);

>      }

>

>    if (alt_dump_file && (dump_kind & alt_flags))

>      {

> -      dump_loc (dump_kind, alt_dump_file, loc);

> +      dump_loc (dump_kind, alt_dump_file, srcloc);

>        print_gimple_stmt (alt_dump_file, gs, spc, dump_flags | extra_dump_flags);

>      }

>  }

> @@ -423,27 +470,6 @@ dump_generic_expr (dump_flags_t dump_kind, dump_flags_t extra_dump_flags,

>        print_generic_expr (alt_dump_file, t, dump_flags | extra_dump_flags);

>  }

>

> -

> -/* Similar to dump_generic_expr, except additionally print the source

> -   location.  */

> -

> -void

> -dump_generic_expr_loc (dump_flags_t dump_kind, source_location loc,

> -                      dump_flags_t extra_dump_flags, tree t)

> -{

> -  if (dump_file && (dump_kind & pflags))

> -    {

> -      dump_loc (dump_kind, dump_file, loc);

> -      print_generic_expr (dump_file, t, dump_flags | extra_dump_flags);

> -    }

> -

> -  if (alt_dump_file && (dump_kind & alt_flags))

> -    {

> -      dump_loc (dump_kind, alt_dump_file, loc);

> -      print_generic_expr (alt_dump_file, t, dump_flags | extra_dump_flags);

> -    }

> -}

> -

>  /* Output a formatted message using FORMAT on appropriate dump streams.  */

>

>  void

> @@ -469,13 +495,14 @@ dump_printf (dump_flags_t dump_kind, const char *format, ...)

>  /* Similar to dump_printf, except source location is also printed.  */

>

>  void

> -dump_printf_loc (dump_flags_t dump_kind, source_location loc,

> +dump_printf_loc (dump_flags_t dump_kind, const dump_location_t &loc,

>                  const char *format, ...)

>  {

> +  location_t srcloc = loc.get_location_t ();

>    if (dump_file && (dump_kind & pflags))

>      {

>        va_list ap;

> -      dump_loc (dump_kind, dump_file, loc);

> +      dump_loc (dump_kind, dump_file, srcloc);

>        va_start (ap, format);

>        vfprintf (dump_file, format, ap);

>        va_end (ap);

> @@ -484,7 +511,7 @@ dump_printf_loc (dump_flags_t dump_kind, source_location loc,

>    if (alt_dump_file && (dump_kind & alt_flags))

>      {

>        va_list ap;

> -      dump_loc (dump_kind, alt_dump_file, loc);

> +      dump_loc (dump_kind, alt_dump_file, srcloc);

>        va_start (ap, format);

>        vfprintf (alt_dump_file, format, ap);

>        va_end (ap);

> @@ -1059,3 +1086,53 @@ enable_rtl_dump_file (void)

>                             NULL);

>    return num_enabled > 0;

>  }

> +

> +#if CHECKING_P

> +

> +namespace selftest {

> +

> +/* Verify that the dump_location_t constructors capture the source location

> +   at which they were called (provided that the build compiler is sufficiently

> +   recent).  */

> +

> +static void

> +test_impl_location ()

> +{

> +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)

> +  /* Default ctor.  */

> +  {

> +    dump_location_t loc;

> +    const int expected_line = __LINE__ - 1;

> +    ASSERT_STR_CONTAINS (loc.get_impl_location ().m_file, "dumpfile.c");

> +    ASSERT_EQ (loc.get_impl_location ().m_line, expected_line);

> +  }

> +

> +  /* Constructing from a gimple.  */

> +  {

> +    dump_location_t loc ((gimple *)NULL);

> +    const int expected_line = __LINE__ - 1;

> +    ASSERT_STR_CONTAINS (loc.get_impl_location ().m_file, "dumpfile.c");

> +    ASSERT_EQ (loc.get_impl_location ().m_line, expected_line);

> +  }

> +

> +  /* Constructing from an rtx_insn.  */

> +  {

> +    dump_location_t loc ((rtx_insn *)NULL);

> +    const int expected_line = __LINE__ - 1;

> +    ASSERT_STR_CONTAINS (loc.get_impl_location ().m_file, "dumpfile.c");

> +    ASSERT_EQ (loc.get_impl_location ().m_line, expected_line);

> +  }

> +#endif

> +}

> +

> +/* Run all of the selftests within this file.  */

> +

> +void

> +dumpfile_c_tests ()

> +{

> +  test_impl_location ();

> +}

> +

> +} // namespace selftest

> +

> +#endif /* CHECKING_P */

> diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h

> index f6ad670..90d8930 100644

> --- a/gcc/dumpfile.h

> +++ b/gcc/dumpfile.h

> @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.  If not see

>  #ifndef GCC_DUMPFILE_H

>  #define GCC_DUMPFILE_H 1

>

> +#include "profile-count.h"

>

>  /* Different tree dump places.  When you add new tree dump places,

>     extend the DUMP_FILES array in dumpfile.c.  */

> @@ -268,20 +269,165 @@ struct dump_file_info

>    bool graph_dump_initialized;

>  };

>

> +/* A class for describing where in the user's source that a dump message

> +   relates to, with various constructors for convenience.

> +   In particular, this lets us associate dump messages

> +   with hotness information (e.g. from PGO), allowing them to

> +   be prioritized by code hotness.  */

> +

> +class dump_user_location_t

> +{

> + public:

> +  /* Default constructor, analogous to UNKNOWN_LOCATION.  */

> +  dump_user_location_t () : m_count (), m_loc (UNKNOWN_LOCATION) {}

> +

> +  /* Construct from a gimple statement (using its location and hotness).  */

> +  dump_user_location_t (gimple *stmt);

> +

> +  /* Construct from an RTL instruction (using its location and hotness).  */

> +  dump_user_location_t (rtx_insn *insn);

> +

> +  /* Construct from a location_t.  This one is deprecated (since it doesn't

> +     capture hotness information); it thus needs to be spelled out.  */

> +  static dump_user_location_t

> +  from_location_t (location_t loc)

> +  {

> +    return dump_user_location_t (profile_count (), loc);

> +  }

> +

> +  /* Construct from a function declaration.  This one requires spelling out

> +     to avoid accidentally constructing from other kinds of tree.  */

> +  static dump_user_location_t

> +  from_function_decl (tree fndecl);

> +

> +  profile_count get_count () const { return m_count; }

> +  location_t get_location_t () const { return m_loc; }

> +

> + private:

> +  /* Private ctor from count and location, for use by from_location_t.  */

> +  dump_user_location_t (profile_count count, location_t loc)

> +    : m_count (count), m_loc (loc)

> +  {}

> +

> +  profile_count m_count;

> +  location_t m_loc;

> +};

> +

> +/* A class for identifying where in the compiler's own source

> +   (or a plugin) that a dump message is being emitted from.  */

> +

> +struct dump_impl_location_t

> +{

> +  dump_impl_location_t (

> +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)

> +                       const char *file = __builtin_FILE (),

> +                       int line = __builtin_LINE (),

> +                       const char *function = __builtin_FUNCTION ()

> +#else

> +                       const char *file = __FILE__,

> +                       int line = __LINE__,

> +                       const char *function = NULL

> +#endif

> +  )

> +  : m_file (file), m_line (line), m_function (function)

> +  {}

> +

> +  const char *m_file;

> +  int m_line;

> +  const char *m_function;

> +};

> +

> +/* A bundle of information for describing the location of a dump message:

> +   (a) the source location and hotness within the user's code, together with

> +   (b) the source location within the compiler/plugin.

> +

> +   The constructors use default parameters so that (b) gets sets up

> +   automatically.

> +

> +   The upshot is that you can pass in e.g. a gimple * to dump_printf_loc,

> +   and the dump call will automatically record where in GCC's source

> +   code the dump was emitted from.  */

> +

> +class dump_location_t

> +{

> + public:

> +  /* Default constructor, analogous to UNKNOWN_LOCATION.  */

> +  dump_location_t (const dump_impl_location_t &impl_location

> +                    = dump_impl_location_t ())

> +  : m_user_location (dump_user_location_t ()),

> +    m_impl_location (impl_location)

> +  {

> +  }

> +

> +  /* Construct from a gimple statement (using its location and hotness).  */

> +  dump_location_t (gimple *stmt,

> +                  const dump_impl_location_t &impl_location

> +                    = dump_impl_location_t ())

> +  : m_user_location (dump_user_location_t (stmt)),

> +    m_impl_location (impl_location)

> +  {

> +  }

> +

> +  /* Construct from an RTL instruction (using its location and hotness).  */

> +  dump_location_t (rtx_insn *insn,

> +                  const dump_impl_location_t &impl_location

> +                  = dump_impl_location_t ())

> +  : m_user_location (dump_user_location_t (insn)),

> +    m_impl_location (impl_location)

> +  {

> +  }

> +

> +  /* Construct from a dump_user_location_t.  */

> +  dump_location_t (const dump_user_location_t &user_location,

> +                  const dump_impl_location_t &impl_location

> +                    = dump_impl_location_t ())

> +  : m_user_location (user_location),

> +    m_impl_location (impl_location)

> +  {

> +  }

> +

> +  /* Construct from a location_t.  This one is deprecated (since it doesn't

> +     capture hotness information), and thus requires spelling out.  */

> +  static dump_location_t

> +  from_location_t (location_t loc,

> +                  const dump_impl_location_t &impl_location

> +                    = dump_impl_location_t ())

> +  {

> +    return dump_location_t (dump_user_location_t::from_location_t (loc),

> +                           impl_location);

> +  }

> +

> +  const dump_user_location_t &

> +  get_user_location () const { return m_user_location; }

> +

> +  const dump_impl_location_t &

> +  get_impl_location () const { return m_impl_location; }

> +

> +  location_t get_location_t () const

> +  {

> +    return m_user_location.get_location_t ();

> +  }

> +

> +  profile_count get_count () const { return m_user_location.get_count (); }

> +

> + private:

> +  dump_user_location_t m_user_location;

> +  dump_impl_location_t m_impl_location;

> +};

> +

>  /* In dumpfile.c */

>  extern FILE *dump_begin (int, dump_flags_t *);

>  extern void dump_end (int, FILE *);

>  extern int opt_info_switch_p (const char *);

>  extern const char *dump_flag_name (int);

>  extern void dump_printf (dump_flags_t, const char *, ...) ATTRIBUTE_PRINTF_2;

> -extern void dump_printf_loc (dump_flags_t, source_location,

> -                             const char *, ...) ATTRIBUTE_PRINTF_3;

> +extern void dump_printf_loc (dump_flags_t, const dump_location_t &,

> +                            const char *, ...) ATTRIBUTE_PRINTF_3;

>  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, source_location, dump_flags_t, tree);

>  extern void dump_generic_expr (dump_flags_t, dump_flags_t, tree);

> -extern void dump_gimple_stmt_loc (dump_flags_t, source_location, dump_flags_t,

> -                                 gimple *, int);

> +extern void dump_gimple_stmt_loc (dump_flags_t, const dump_location_t &,

> +                                 dump_flags_t, gimple *, int);

>  extern void dump_gimple_stmt (dump_flags_t, dump_flags_t, gimple *, int);

>  extern void print_combine_total_stats (void);

>  extern bool enable_rtl_dump_file (void);

> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c

> index a01bce7..f12e4a7 100644

> --- a/gcc/gimple-fold.c

> +++ b/gcc/gimple-fold.c

> @@ -347,8 +347,7 @@ fold_gimple_assign (gimple_stmt_iterator *si)

>                   {

>                     if (dump_enabled_p ())

>                       {

> -                       location_t loc = gimple_location_safe (stmt);

> -                       dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

> +                       dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt,

>                                          "resolving virtual function address "

>                                          "reference to function %s\n",

>                                          targets.length () == 1

> @@ -4061,8 +4060,7 @@ gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace)

>               tree lhs = gimple_call_lhs (stmt);

>               if (dump_enabled_p ())

>                 {

> -                 location_t loc = gimple_location_safe (stmt);

> -                 dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

> +                 dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt,

>                                    "folding virtual function call to %s\n",

>                                    targets.length () == 1

>                                    ? targets[0]->name ()

> diff --git a/gcc/gimple-loop-interchange.cc b/gcc/gimple-loop-interchange.cc

> index eb35263..08aeb8e 100644

> --- a/gcc/gimple-loop-interchange.cc

> +++ b/gcc/gimple-loop-interchange.cc

> @@ -523,7 +523,7 @@ loop_cand::analyze_iloop_reduction_var (tree var)

>

>    /* Handle and verify a series of stmts feeding the reduction op.  */

>    if (single_use != next_def

> -      && !check_reduction_path (UNKNOWN_LOCATION, m_loop, phi, next,

> +      && !check_reduction_path (dump_user_location_t (), m_loop, phi, next,

>                                 gimple_assign_rhs_code (single_use)))

>      return false;

>

> @@ -1578,7 +1578,7 @@ bool

>  tree_loop_interchange::interchange (vec<data_reference_p> datarefs,

>                                     vec<ddr_p> ddrs)

>  {

> -  location_t loc = find_loop_location (m_loop_nest[0]);

> +  dump_user_location_t loc = find_loop_location (m_loop_nest[0]);

>    bool changed_p = false;

>    /* In each iteration we try to interchange I-th loop with (I+1)-th loop.

>       The overall effect is to push inner loop to outermost level in whole

> diff --git a/gcc/graphite-isl-ast-to-gimple.c b/gcc/graphite-isl-ast-to-gimple.c

> index b607b12..9e78465 100644

> --- a/gcc/graphite-isl-ast-to-gimple.c

> +++ b/gcc/graphite-isl-ast-to-gimple.c

> @@ -1409,7 +1409,7 @@ scop_to_isl_ast (scop_p scop)

>    isl_ctx_set_max_operations (scop->isl_context, old_max_operations);

>    if (isl_ctx_last_error (scop->isl_context) != isl_error_none)

>      {

> -      location_t loc = find_loop_location

> +      dump_user_location_t loc = find_loop_location

>         (scop->scop_info->region.entry->dest->loop_father);

>        if (isl_ctx_last_error (scop->isl_context) == isl_error_quota)

>         dump_printf_loc (MSG_MISSED_OPTIMIZATION, loc,

> @@ -1518,7 +1518,7 @@ graphite_regenerate_ast_isl (scop_p scop)

>

>    if (t.codegen_error_p ())

>      {

> -      location_t loc = find_loop_location

> +      dump_user_location_t loc = find_loop_location

>         (scop->scop_info->region.entry->dest->loop_father);

>        dump_printf_loc (MSG_MISSED_OPTIMIZATION, loc,

>                        "loop nest not optimized, code generation error\n");

> diff --git a/gcc/graphite-optimize-isl.c b/gcc/graphite-optimize-isl.c

> index 456a797..35e9ac0 100644

> --- a/gcc/graphite-optimize-isl.c

> +++ b/gcc/graphite-optimize-isl.c

> @@ -160,7 +160,7 @@ optimize_isl (scop_p scop)

>    if (!scop->transformed_schedule

>        || isl_ctx_last_error (scop->isl_context) != isl_error_none)

>      {

> -      location_t loc = find_loop_location

> +      dump_user_location_t loc = find_loop_location

>         (scop->scop_info->region.entry->dest->loop_father);

>        if (isl_ctx_last_error (scop->isl_context) == isl_error_quota)

>         dump_printf_loc (MSG_MISSED_OPTIMIZATION, loc,

> @@ -182,7 +182,7 @@ optimize_isl (scop_p scop)

>

>    if (same_schedule)

>      {

> -      location_t loc = find_loop_location

> +      dump_user_location_t loc = find_loop_location

>         (scop->scop_info->region.entry->dest->loop_father);

>        dump_printf_loc (MSG_NOTE, loc,

>                        "loop nest not optimized, optimized schedule is "

> diff --git a/gcc/graphite.c b/gcc/graphite.c

> index bcf4828..ddf16a8 100644

> --- a/gcc/graphite.c

> +++ b/gcc/graphite.c

> @@ -412,7 +412,7 @@ graphite_transform_loops (void)

>         changed = true;

>         if (graphite_regenerate_ast_isl (scop))

>           {

> -           location_t loc = find_loop_location

> +           dump_user_location_t loc = find_loop_location

>               (scops[i]->scop_info->region.entry->dest->loop_father);

>             dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

>                              "loop nest optimized\n");

> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c

> index 308b6e6..e99d8cc 100644

> --- a/gcc/ipa-devirt.c

> +++ b/gcc/ipa-devirt.c

> @@ -3755,8 +3755,7 @@ ipa_devirt (void)

>               {

>                 if (dump_enabled_p ())

>                    {

> -                    location_t locus = gimple_location_safe (e->call_stmt);

> -                    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, locus,

> +                    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, e->call_stmt,

>                                      "speculatively devirtualizing call "

>                                      "in %s to %s\n",

>                                      n->dump_name (),

> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c

> index 000c05f..8b19fe3 100644

> --- a/gcc/ipa-prop.c

> +++ b/gcc/ipa-prop.c

> @@ -2842,8 +2842,7 @@ ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target,

>             {

>               if (dump_enabled_p ())

>                 {

> -                 location_t loc = gimple_location_safe (ie->call_stmt);

> -                 dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

> +                 dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, ie->call_stmt,

>                                    "discovered direct call non-invariant %s\n",

>                                    ie->caller->dump_name ());

>                 }

> @@ -2853,8 +2852,7 @@ ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target,

>

>            if (dump_enabled_p ())

>             {

> -             location_t loc = gimple_location_safe (ie->call_stmt);

> -             dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

> +             dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, ie->call_stmt,

>                                "discovered direct call to non-function in %s, "

>                                "making it __builtin_unreachable\n",

>                                ie->caller->dump_name ());

> @@ -2942,9 +2940,7 @@ ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target,

>       }

>    if (dump_enabled_p ())

>      {

> -      location_t loc = gimple_location_safe (ie->call_stmt);

> -

> -      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

> +      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, ie->call_stmt,

>                        "converting indirect call in %s to direct call to %s\n",

>                        ie->caller->name (), callee->name ());

>      }

> diff --git a/gcc/ipa.c b/gcc/ipa.c

> index 82fc334..3b6b5e5 100644

> --- a/gcc/ipa.c

> +++ b/gcc/ipa.c

> @@ -225,13 +225,8 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets,

>                        (builtin_decl_implicit (BUILT_IN_UNREACHABLE));

>

>           if (dump_enabled_p ())

> -            {

> -             location_t locus;

> -             if (edge->call_stmt)

> -               locus = gimple_location (edge->call_stmt);

> -             else

> -               locus = UNKNOWN_LOCATION;

> -             dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, locus,

> +           {

> +             dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, edge->call_stmt,

>                                "devirtualizing call in %s to %s\n",

>                                edge->caller->dump_name (),

>                                target->dump_name ());

> diff --git a/gcc/loop-unroll.c b/gcc/loop-unroll.c

> index 5a03932..48bbda0 100644

> --- a/gcc/loop-unroll.c

> +++ b/gcc/loop-unroll.c

> @@ -189,7 +189,7 @@ static rtx get_expansion (struct var_to_expand *);

>     appropriate given the dump or -fopt-info settings.  */

>

>  static void

> -report_unroll (struct loop *loop, location_t locus)

> +report_unroll (struct loop *loop, dump_location_t locus)

>  {

>    dump_flags_t report_flags = MSG_OPTIMIZED_LOCATIONS | TDF_DETAILS;

>

> @@ -220,7 +220,7 @@ decide_unrolling (int flags)

>    FOR_EACH_LOOP (loop, LI_FROM_INNERMOST)

>      {

>        loop->lpt_decision.decision = LPT_NONE;

> -      location_t locus = get_loop_location (loop);

> +      dump_user_location_t locus = get_loop_location (loop);

>

>        if (dump_enabled_p ())

>         dump_printf_loc (MSG_NOTE, locus,

> diff --git a/gcc/omp-grid.c b/gcc/omp-grid.c

> index ffa301e..6edc92f 100644

> --- a/gcc/omp-grid.c

> +++ b/gcc/omp-grid.c

> @@ -91,7 +91,7 @@ struct grid_prop

>    bool tiling;

>    /* Location of the target construct for optimization information

>       messages.  */

> -  location_t target_loc;

> +  dump_user_location_t target_loc;

>    /* The collapse clause of the involved loops.  Collapse value of all of them

>       must be the same for gridification to take place.  */

>    size_t collapse;

> @@ -177,10 +177,10 @@ grid_find_single_omp_among_assignments_1 (gimple_seq seq, grid_prop *grid,

>                                    GRID_MISSED_MSG_PREFIX "%s construct "

>                                    "contains multiple OpenMP constructs\n",

>                                    name);

> -                 dump_printf_loc (MSG_NOTE, gimple_location (*ret),

> +                 dump_printf_loc (MSG_NOTE, *ret,

>                                    "The first OpenMP construct within "

>                                    "a parallel\n");

> -                 dump_printf_loc (MSG_NOTE, gimple_location (stmt),

> +                 dump_printf_loc (MSG_NOTE, stmt,

>                                    "The second OpenMP construct within "

>                                    "a parallel\n");

>                 }

> @@ -195,7 +195,7 @@ grid_find_single_omp_among_assignments_1 (gimple_seq seq, grid_prop *grid,

>               dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,

>                                GRID_MISSED_MSG_PREFIX "%s construct contains "

>                                "a complex statement\n", name);

> -             dump_printf_loc (MSG_NOTE, gimple_location (stmt),

> +             dump_printf_loc (MSG_NOTE, stmt,

>                                "This statement cannot be analyzed for "

>                                "gridification\n");

>             }

> @@ -286,7 +286,7 @@ grid_find_ungridifiable_statement (gimple_stmt_iterator *gsi,

>     loop that is evaluated for possible gridification.  */

>

>  static bool

> -grid_parallel_clauses_gridifiable (gomp_parallel *par, location_t tloc)

> +grid_parallel_clauses_gridifiable (gomp_parallel *par, dump_user_location_t tloc)

>  {

>    tree clauses = gimple_omp_parallel_clauses (par);

>    while (clauses)

> @@ -300,7 +300,7 @@ grid_parallel_clauses_gridifiable (gomp_parallel *par, location_t tloc)

>                                GRID_MISSED_MSG_PREFIX "because there is "

>                                "a num_threads clause of the parallel "

>                                "construct\n");

> -             dump_printf_loc (MSG_NOTE, gimple_location (par),

> +             dump_printf_loc (MSG_NOTE, par,

>                                "Parallel construct has a num_threads clause\n");

>             }

>           return false;

> @@ -311,7 +311,7 @@ grid_parallel_clauses_gridifiable (gomp_parallel *par, location_t tloc)

>               dump_printf_loc (MSG_MISSED_OPTIMIZATION, tloc,

>                                GRID_MISSED_MSG_PREFIX "a reduction clause "

>                                "is present\n ");

> -             dump_printf_loc (MSG_NOTE, gimple_location (par),

> +             dump_printf_loc (MSG_NOTE, par,

>                                "Parallel construct has a reduction clause\n");

>             }

>           return false;

> @@ -341,7 +341,7 @@ grid_inner_loop_gridifiable_p (gomp_for *gfor, grid_prop *grid)

>                            GRID_MISSED_MSG_PREFIX "the inner loop "

>                            "loop bounds computation contains a complex "

>                            "statement\n");

> -         dump_printf_loc (MSG_NOTE, gimple_location (gfor),

> +         dump_printf_loc (MSG_NOTE, gfor,

>                            "Loop construct cannot be analyzed for "

>                            "gridification\n");

>         }

> @@ -361,7 +361,7 @@ grid_inner_loop_gridifiable_p (gomp_for *gfor, grid_prop *grid)

>                   dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,

>                                    GRID_MISSED_MSG_PREFIX "the inner loop "

>                                    "has a non-automatic schedule clause\n");

> -                 dump_printf_loc (MSG_NOTE, gimple_location (gfor),

> +                 dump_printf_loc (MSG_NOTE, gfor,

>                                    "Loop construct has a non automatic "

>                                    "schedule clause\n");

>                 }

> @@ -375,7 +375,7 @@ grid_inner_loop_gridifiable_p (gomp_for *gfor, grid_prop *grid)

>               dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,

>                                GRID_MISSED_MSG_PREFIX "a reduction "

>                                "clause is present\n ");

> -             dump_printf_loc (MSG_NOTE, gimple_location (gfor),

> +             dump_printf_loc (MSG_NOTE, gfor,

>                                "Loop construct has a reduction schedule "

>                                "clause\n");

>             }

> @@ -404,7 +404,7 @@ grid_inner_loop_gridifiable_p (gomp_for *gfor, grid_prop *grid)

>                              GRID_MISSED_MSG_PREFIX "the inner loop contains "

>                              "statement %s which cannot be transformed\n",

>                              gimple_code_name[(int) gimple_code (bad)]);

> -         dump_printf_loc (MSG_NOTE, gimple_location (bad),

> +         dump_printf_loc (MSG_NOTE, bad,

>                            "This statement cannot be analyzed for "

>                            "gridification\n");

>         }

> @@ -422,7 +422,7 @@ grid_inner_loop_gridifiable_p (gomp_for *gfor, grid_prop *grid)

>  static bool

>  grid_dist_follows_simple_pattern (gomp_for *dist, grid_prop *grid)

>  {

> -  location_t tloc = grid->target_loc;

> +  dump_user_location_t tloc = grid->target_loc;

>    gimple *stmt = grid_find_single_omp_among_assignments (gimple_omp_body (dist),

>                                                          grid, "distribute");

>    gomp_parallel *par;

> @@ -468,7 +468,7 @@ grid_gfor_follows_tiling_pattern (gomp_for *gfor, grid_prop *grid)

>           dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,

>                            GRID_MISSED_MSG_PREFIX "an inner loop is not "

>                            "a simple for loop\n");

> -         dump_printf_loc (MSG_NOTE, gimple_location (gfor),

> +         dump_printf_loc (MSG_NOTE, gfor,

>                            "This statement is not a simple for loop\n");

>         }

>        return false;

> @@ -484,7 +484,7 @@ grid_gfor_follows_tiling_pattern (gomp_for *gfor, grid_prop *grid)

>           dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,

>                            GRID_MISSED_MSG_PREFIX "an inner loop does not "

>                            "have use the same collapse clause\n");

> -         dump_printf_loc (MSG_NOTE, gimple_location (gfor),

> +         dump_printf_loc (MSG_NOTE, gfor,

>                            "Loop construct uses a different collapse clause\n");

>         }

>        return false;

> @@ -524,7 +524,7 @@ grid_gfor_follows_tiling_pattern (gomp_for *gfor, grid_prop *grid)

>               dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,

>                                GRID_MISSED_MSG_PREFIX "the distribute and "

>                                "an internal loop do not agree on tile size\n");

> -             dump_printf_loc (MSG_NOTE, gimple_location (gfor),

> +             dump_printf_loc (MSG_NOTE, gfor,

>                                "Loop construct does not seem to loop over "

>                                "a tile size\n");

>             }

> @@ -636,7 +636,7 @@ grid_dist_follows_tiling_pattern (gimple_seq seq, grid_prop *grid,

>                   dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,

>                                    GRID_MISSED_MSG_PREFIX "the distribute "

>                                    "construct contains a try..catch region\n");

> -                 dump_printf_loc (MSG_NOTE, gimple_location (try_stmt),

> +                 dump_printf_loc (MSG_NOTE, try_stmt,

>                                    "This statement cannot be analyzed for "

>                                    "tiled gridification\n");

>                 }

> @@ -661,7 +661,7 @@ grid_dist_follows_tiling_pattern (gimple_seq seq, grid_prop *grid,

>               dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,

>                                GRID_MISSED_MSG_PREFIX "the distribute "

>                                "construct contains a call\n");

> -             dump_printf_loc (MSG_NOTE, gimple_location (stmt),

> +             dump_printf_loc (MSG_NOTE, stmt,

>                                "This statement cannot be analyzed for "

>                                "tiled gridification\n");

>             }

> @@ -677,7 +677,7 @@ grid_dist_follows_tiling_pattern (gimple_seq seq, grid_prop *grid,

>                                    GRID_MISSED_MSG_PREFIX "a parallel "

>                                    "construct contains another parallel "

>                                    "construct\n");

> -                 dump_printf_loc (MSG_NOTE, gimple_location (stmt),

> +                 dump_printf_loc (MSG_NOTE, stmt,

>                                    "This parallel construct is nested in "

>                                    "another one\n");

>                 }

> @@ -698,7 +698,7 @@ grid_dist_follows_tiling_pattern (gimple_seq seq, grid_prop *grid,

>                                    GRID_MISSED_MSG_PREFIX "a loop "

>                                    "construct is not nested within a parallel "

>                                    "construct\n");

> -                 dump_printf_loc (MSG_NOTE, gimple_location (stmt),

> +                 dump_printf_loc (MSG_NOTE, stmt,

>                                    "This loop construct is not nested in "

>                                    "a parallel construct\n");

>                 }

> @@ -714,7 +714,7 @@ grid_dist_follows_tiling_pattern (gimple_seq seq, grid_prop *grid,

>               dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,

>                                GRID_MISSED_MSG_PREFIX "the distribute "

>                                "construct contains a complex statement\n");

> -             dump_printf_loc (MSG_NOTE, gimple_location (stmt),

> +             dump_printf_loc (MSG_NOTE, stmt,

>                                "This statement cannot be analyzed for "

>                                "tiled gridification\n");

>             }

> @@ -734,7 +734,7 @@ grid_target_follows_gridifiable_pattern (gomp_target *target, grid_prop *grid)

>    if (gimple_omp_target_kind (target) != GF_OMP_TARGET_KIND_REGION)

>      return false;

>

> -  location_t tloc = gimple_location (target);

> +  dump_user_location_t tloc = target;

>    grid->target_loc = tloc;

>    gimple *stmt

>      = grid_find_single_omp_among_assignments (gimple_omp_body (target),

> @@ -1257,14 +1257,13 @@ grid_attempt_target_gridification (gomp_target *target,

>                                    gbind *tgt_bind)

>  {

>    /* removed group_size */

> -  grid_prop grid;

> -  memset (&grid, 0, sizeof (grid));

> +  grid_prop grid = {};

>    if (!target || !grid_target_follows_gridifiable_pattern (target, &grid))

>      return;

>

>    location_t loc = gimple_location (target);

>    if (dump_enabled_p ())

> -    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

> +    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, target,

>                      "Target construct will be turned into a gridified HSA "

>                      "kernel\n");

>

> diff --git a/gcc/profile.c b/gcc/profile.c

> index 8ba6dc7..0cd0270 100644

> --- a/gcc/profile.c

> +++ b/gcc/profile.c

> @@ -447,9 +447,14 @@ read_profile_edge_counts (gcov_type *exec_counts)

>                       {

>                         static bool informed = 0;

>                         if (dump_enabled_p () && !informed)

> -                         dump_printf_loc (MSG_NOTE, input_location,

> -                                           "corrupted profile info: edge count"

> -                                           " exceeds maximal count\n");

> +                         {

> +                           dump_location_t loc

> +                             = dump_location_t::from_location_t

> +                               (input_location);

> +                           dump_printf_loc (MSG_NOTE, loc,

> +                                            "corrupted profile info: edge count"

> +                                            " exceeds maximal count\n");

> +                         }

>                         informed = 1;

>                       }

>                     else

> @@ -672,7 +677,8 @@ compute_branch_probabilities (unsigned cfg_checksum, unsigned lineno_checksum)

>           if (dump_enabled_p () && informed == 0)

>             {

>               informed = 1;

> -             dump_printf_loc (MSG_NOTE, input_location,

> +             dump_printf_loc (MSG_NOTE,

> +                             dump_location_t::from_location_t (input_location),

>                                "correcting inconsistent profile data\n");

>             }

>           correct_negative_edge_counts ();

> diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c

> index fe221ff..a9aacc02 100644

> --- a/gcc/selftest-run-tests.c

> +++ b/gcc/selftest-run-tests.c

> @@ -59,6 +59,7 @@ selftest::run_tests ()

>    /* Low-level data structures.  */

>    bitmap_c_tests ();

>    sbitmap_c_tests ();

> +  dumpfile_c_tests ();

>    et_forest_c_tests ();

>    hash_map_tests_c_tests ();

>    hash_set_tests_c_tests ();

> diff --git a/gcc/selftest.h b/gcc/selftest.h

> index fc47b2c..a5507cc 100644

> --- a/gcc/selftest.h

> +++ b/gcc/selftest.h

> @@ -188,6 +188,7 @@ extern void attribute_c_tests ();

>  extern void bitmap_c_tests ();

>  extern void diagnostic_c_tests ();

>  extern void diagnostic_show_locus_c_tests ();

> +extern void dumpfile_c_tests ();

>  extern void edit_context_c_tests ();

>  extern void et_forest_c_tests ();

>  extern void fibonacci_heap_c_tests ();

> diff --git a/gcc/tree-loop-distribution.c b/gcc/tree-loop-distribution.c

> index c6e0a60..1206614 100644

> --- a/gcc/tree-loop-distribution.c

> +++ b/gcc/tree-loop-distribution.c

> @@ -3117,7 +3117,7 @@ pass_loop_distribution::execute (function *fun)

>             break;

>

>           const char *str = loop->inner ? " nest" : "";

> -         location_t loc = find_loop_location (loop);

> +         dump_user_location_t loc = find_loop_location (loop);

>           if (!cd)

>             {

>               calculate_dominance_info (CDI_DOMINATORS);

> diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c

> index c49f032..e79a954 100644

> --- a/gcc/tree-parloops.c

> +++ b/gcc/tree-parloops.c

> @@ -3286,7 +3286,6 @@ parallelize_loops (bool oacc_kernels_p)

>    struct tree_niter_desc niter_desc;

>    struct obstack parloop_obstack;

>    HOST_WIDE_INT estimated;

> -  source_location loop_loc;

>

>    /* Do not parallelize loops in the functions created by parallelization.  */

>    if (!oacc_kernels_p

> @@ -3411,7 +3410,7 @@ parallelize_loops (bool oacc_kernels_p)

>        changed = true;

>        skip_loop = loop->inner;

>

> -      loop_loc = find_loop_location (loop);

> +      dump_user_location_t loop_loc = find_loop_location (loop);

>        if (loop->inner)

>         dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loop_loc,

>                          "parallelizing outer loop %d\n", loop->num);

> diff --git a/gcc/tree-ssa-loop-ivcanon.c b/gcc/tree-ssa-loop-ivcanon.c

> index 24bf60e..5f741c3 100644

> --- a/gcc/tree-ssa-loop-ivcanon.c

> +++ b/gcc/tree-ssa-loop-ivcanon.c

> @@ -691,7 +691,7 @@ try_unroll_loop_completely (struct loop *loop,

>                             edge exit, tree niter, bool may_be_zero,

>                             enum unroll_level ul,

>                             HOST_WIDE_INT maxiter,

> -                           location_t locus, bool allow_peel)

> +                           dump_user_location_t locus, bool allow_peel)

>  {

>    unsigned HOST_WIDE_INT n_unroll = 0;

>    bool n_unroll_found = false;

> @@ -1162,7 +1162,7 @@ canonicalize_loop_induction_variables (struct loop *loop,

>    tree niter;

>    HOST_WIDE_INT maxiter;

>    bool modified = false;

> -  location_t locus = UNKNOWN_LOCATION;

> +  dump_user_location_t locus;

>    struct tree_niter_desc niter_desc;

>    bool may_be_zero = false;

>

> @@ -1177,7 +1177,7 @@ canonicalize_loop_induction_variables (struct loop *loop,

>         = niter_desc.may_be_zero && !integer_zerop (niter_desc.may_be_zero);

>      }

>    if (TREE_CODE (niter) == INTEGER_CST)

> -    locus = gimple_location (last_stmt (exit->src));

> +    locus = last_stmt (exit->src);

>    else

>      {

>        /* For non-constant niter fold may_be_zero into niter again.  */

> @@ -1204,7 +1204,7 @@ canonicalize_loop_induction_variables (struct loop *loop,

>         niter = find_loop_niter_by_eval (loop, &exit);

>

>        if (exit)

> -        locus = gimple_location (last_stmt (exit->src));

> +        locus = last_stmt (exit->src);

>

>        if (TREE_CODE (niter) != INTEGER_CST)

>         exit = NULL;

> diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c

> index 519649a..6b445bd 100644

> --- a/gcc/tree-ssa-loop-ivopts.c

> +++ b/gcc/tree-ssa-loop-ivopts.c

> @@ -7535,7 +7535,7 @@ tree_ssa_iv_optimize_loop (struct ivopts_data *data, struct loop *loop)

>

>    gcc_assert (!data->niters);

>    data->current_loop = loop;

> -  data->loop_loc = find_loop_location (loop);

> +  data->loop_loc = find_loop_location (loop).get_location_t ();

>    data->speed = optimize_loop_for_speed_p (loop);

>

>    if (dump_file && (dump_flags & TDF_DETAILS))

> diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c

> index f5ffc0f..03588a0 100644

> --- a/gcc/tree-ssa-loop-niter.c

> +++ b/gcc/tree-ssa-loop-niter.c

> @@ -2627,7 +2627,7 @@ number_of_iterations_exit (struct loop *loop, edge exit,

>      return true;

>

>    if (warn)

> -    dump_printf_loc (MSG_MISSED_OPTIMIZATION, gimple_location_safe (stmt),

> +    dump_printf_loc (MSG_MISSED_OPTIMIZATION, stmt,

>                      "missed loop optimization: niters analysis ends up "

>                      "with assumptions.\n");

>

> diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c

> index 3d025c2..e5eddf9 100644

> --- a/gcc/tree-ssa-sccvn.c

> +++ b/gcc/tree-ssa-sccvn.c

> @@ -5866,8 +5866,7 @@ eliminate_dom_walker::before_dom_children (basic_block b)

>                     fn = builtin_decl_implicit (BUILT_IN_UNREACHABLE);

>                   if (dump_enabled_p ())

>                     {

> -                     location_t loc = gimple_location (stmt);

> -                     dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,

> +                     dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt,

>                                        "converting indirect call to "

>                                        "function %s\n",

>                                        lang_hooks.decl_printable_name (fn, 2));

> diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
David Malcolm June 28, 2018, 2:29 p.m. | #2
On Thu, 2018-06-28 at 13:29 +0200, Richard Biener wrote:
> On Tue, Jun 26, 2018 at 3:54 PM David Malcolm <dmalcolm@redhat.com>

> wrote:

> > 

> > On Mon, 2018-06-25 at 15:34 +0200, Richard Biener wrote:

> > > On Wed, Jun 20, 2018 at 6:34 PM David Malcolm <dmalcolm@redhat.co

> > > m>

> > > wrote:

> > > > 

> > > > Here's v3 of the patch (one big patch this time, rather than a

> > > > kit).

> > > > 

> > > > Like the v2 patch kit, this patch reuses the existing dump API,

> > > > rather than inventing its own.

> > > > 

> > > > Specifically, it uses the dump_* functions in dumpfile.h that

> > > > don't

> > > > take a FILE *, the ones that implicitly write to dump_file

> > > > and/or

> > > > alt_dump_file.  I needed a name for them, so I've taken to

> > > > calling

> > > > them the "structured dump API" (better name ideas welcome).

> > > > 

> > > > v3 eliminates v2's optinfo_guard class, instead using

> > > > "dump_*_loc"

> > > > calls as delimiters when consolidating "dump_*" calls.  There's

> > > > a

> > > > new dump_context class which has responsibility for

> > > > consolidating

> > > > them into optimization records.

> > > > 

> > > > The dump_*_loc calls now capture more than just a location_t:

> > > > they

> > > > capture the profile_count and the location in GCC's own sources

> > > > where

> > > > the dump is being emitted from.

> > > > 

> > > > This works by introducing a new "dump_location_t" class as the

> > > > argument of those dump_*_loc calls.  The dump_location_t can

> > > > be constructed from a gimple * or from an rtx_insn *, so that

> > > > rather than writing:

> > > > 

> > > >   dump_printf_loc (MSG_NOTE, gimple_location (stmt),

> > > >                    "some message: %i", 42);

> > > > 

> > > > you can write:

> > > > 

> > > >   dump_printf_loc (MSG_NOTE, stmt,

> > > >                    "some message: %i", 42);

> > > > 

> > > > and the dump_location_t constructor will grab the location_t

> > > > and

> > > > profile_count of stmt, and the location of the

> > > > "dump_printf_loc"

> > > > callsite (and gracefully handle "stmt" being NULL).

> > > > 

> > > > Earlier versions of the patch captured the location of the

> > > > dump_*_loc call via preprocessor hacks, or didn't work

> > > > properly;

> > > > this version of the patch works more cleanly: internally,

> > > > dump_location_t is split into two new classes:

> > > >   * dump_user_location_t: the location_t and profile_count

> > > > within

> > > >     the *user's code*, and

> > > >   * dump_impl_location_t: the __builtin_FILE/LINE/FUNCTION

> > > > within

> > > >     the *implementation* code (i.e. GCC or a plugin), captured

> > > >     "automagically" via default params

> > > > 

> > > > These classes are sometimes used elsewhere in the code.  For

> > > > example, "vect_location" becomes a dump_user_location_t

> > > > (location_t and profile_count), so that in e.g:

> > > > 

> > > >   vect_location = find_loop_location (loop);

> > > > 

> > > > it's capturing the location_t and profile_count, and then when

> > > > it's used here:

> > > > 

> > > >   dump_printf_loc (MSG_NOTE, vect_location, "foo");

> > > > 

> > > > the dump_location_t is constructed from the vect_location

> > > > plus the dump_impl_location_t at that callsite.

> > > > 

> > > > In contrast, loop-unroll.c's report_unroll's "locus" param

> > > > becomes a dump_location_t: we're interested in where it was

> > > > called from, not in the locations of the various dump_*_loc

> > > > calls

> > > > within it.

> > > > 

> > > > Previous versions of the patch captured a gimple *, and needed

> > > > GTY markers; in this patch, the dump_user_location_t is now

> > > > just a

> > > > location_t and a profile_count.

> > > > 

> > > > The v2 patch added an overload for dump_printf_loc so that you

> > > > could pass in either a location_t, or the new type; this

> > > > version

> > > > of the patch eliminates that: they all now take

> > > > dump_location_t.

> > > > 

> > > > Doing so required adding support for rtx_insn *, so that one

> > > > can

> > > > write this kind of thing in RTL passes:

> > > > 

> > > >   dump_printf_loc (MSG_NOTE, insn, "foo");

> > > > 

> > > > One knock-on effect is that get_loop_location now returns a

> > > > dump_user_location_t rather than a location_t, so that it has

> > > > hotness information.

> > > > 

> > > > Richi: would you like me to split out this location-handling

> > > > code into a separate patch?  (It's kind of redundant without

> > > > adding the remarks and optimization records work, but if that's

> > > > easier I can do it)

> > > 

> > > I think that would be easier because it doesn't require the JSON

> > > stuff and so I'll happily approve it.

> > > 

> > > Thus - trying to review that bits (and sorry for the delay).

> > > 

> > > +  location_t srcloc = loc.get_location_t ();

> > > +

> > >    if (dump_file && (dump_kind & pflags))

> > >      {

> > > -      dump_loc (dump_kind, dump_file, loc);

> > > +      dump_loc (dump_kind, dump_file, srcloc);

> > >        print_gimple_stmt (dump_file, gs, spc, dump_flags |

> > > extra_dump_flags);

> > >      }

> > > 

> > >    if (alt_dump_file && (dump_kind & alt_flags))

> > >      {

> > > -      dump_loc (dump_kind, alt_dump_file, loc);

> > > +      dump_loc (dump_kind, alt_dump_file, srcloc);

> > >        print_gimple_stmt (alt_dump_file, gs, spc, dump_flags |

> > > extra_dump_flags);

> > >      }

> > > +

> > > +  if (optinfo_enabled_p ())

> > > +    {

> > > +      optinfo &info = begin_next_optinfo (loc);

> > > +      info.handle_dump_file_kind (dump_kind);

> > > +      info.add_stmt (gs, extra_dump_flags);

> > > +    }

> > > 

> > > seeing this in multiple places.  I seem to remember that

> > > dump_file / alt_dump_file was suposed to handle dumping

> > > into two locations - a dump file and optinfo (or stdout).  This

> > > looks

> > > like the optinfo "stream" is even more separate.  Could that

> > > obsolete the alt_dump_file stream?  I'd need to review existing

> > > stuff

> > > in more detail to answer but maybe you already know from recently

> > > digging into this.

> > 

> > Possibly.  I attempted this in v1 of the patch, but it was mixed in

> > with

> > a bunch of other stuff.  I'll have another go at doing this.

> > 

> > > Oh, and all the if (optinfo_enable_p ()) stuff is for the

> > > followup

> > > then, right?

> > 

> > Yes.

> > 

> > > I like the boiler-plate changes to dump_* using stuff a lot, so

> > > the

> > > infrastructure to do that (the location wrapping) and these

> > > boiler-

> > > plate

> > > changes are pre-approved if split out.

> > 

> > Thanks.  I split out the location wrapping, and have committed it

> > to

> > trunk (r262149).  It's not clear to me exactly what other parts you

> > liked,

> > so I'm going to try to split out more of the non-JSON bits in the

> > hope that some parts are good enough as-is, and I'll post them for

> > review

> > as followups.

> > 

> > For reference, here's what I've committed (I added some obvious

> > changes

> > to doc/optinfo.texi).

> > 

> > > I think the *_REMARK stuff should get attention of the respective

> > > maintainers - not sure what the difference between NOTE and

> > > REMARK

> > > is ;)

> > 

> > Me neither :)  I think it might come down to "this is purely a

> > debugging

> > message for a pass maintainer" (MSG_NOTE) vs "this is a high-level

> > thing

> > that an advanced user want to see" (MSG_REMARK???).

> > 

> > One idea that occurred to me: are we overusing dump_flags_t?  It's

> > a

> > mixture of TDF_* bitfields (used by our internal dumps) plus MSG_*

> > bitfields (used with -fopt-info).  IIRC the only TDF_ bitfield

> > that's

> > ever used with the MSG_* bitfields is TDF_DETAILS.  Might it make

> > sense

> > to split out the MSG_* bitfields into a different type

> > (dump_level_t???)

> > to reinforce this split?  (and thus the dump_* entrypoints that

> > don't take

> > a FILE * would take this new type).  I'm not sure if this is a good

> > idea

> > or not.

> 

> Making it even more complicated doesn't make it easier to use it

> "correctly".  So I'd rather try to simplify it.  How passes use

> TDF_DETAILS vs. non-details is already highly inconsistent.  This

> is why I liked the original optinfo work because it somehow made

> user-interesting vs. developer-interesting with the same API

> (and to the same dump-file).  Just that MSG_NOTE is exposed to

> users while I think it should be developer-only...

> 

> IMHO the TDF_ stuff should control things at the stmt/tree/BB level,

> thus be IL specific flags while the MSG_ stuff should control

> pass-specific dumping detail.


You mention user-interesting vs developer-interesting, and whether it
would be the same API and/or the same dump-file.

I've posted a bunch of patches here, some small, some large, trying
various different appoaches, coming at the problem from at least
two directions, so maybe it's worth discussing the overall direction
here (with some ASCII art!)

* what is the status quo?
* what do we want to achieve?
* how do we get there?

Status quo:
* We have the "dump_*(MSG_*)" API which hides the destination of
  where we're dumping to (currently dump_file and/or alt_file_file,
  conditionalized via flags).

  dump_* (MSG_*) ----> dumpfile.c --+--> dump_file
                                    |
                                    `--> alt_dump_file

* As of r262149 (the dump_location_t commit) dumpfile.c receives the
  hotness and emission location of each dump_*_loc call, but currently
  it does nothing with that information.
* The messages that we emit through the "dump_*(MSG_*)" API and their
  "levels" are currently rather haphazard.  Some are close to being
  suitable for end-users, but most seem never intended to be
  end-user-facing.  For reference:
    grep -nH -e MSG_MISSED_OPTIMIZATION *.c *.cc | wc -l
    452
    grep -nH -e MSG_NOTE *.c *.cc | wc -l
    551
    grep -nH -e MSG_OPTIMIZED_LOCATIONS *.c *.cc | wc -l
    39
  (though some of these are support code e.g. in dumpfile.c, rather
  than uses).
* The API builds up messages programatically, which is hostile to
  i18n.  The API isn't marked as needing i18n for its strings (which
  IIRC is via using "gmsgid" as a param name being special-cased in
  the gettext toolchain).

What do we want to achieve?
* I want end-users to be able to enable high-level dump messages about
  optimizations, and for those messages to be decipherable without
  reading GCC sources e.g. the opt_problem idea posted here:
  * "[PATCH] [RFC] Higher-level reporting of vectorization problems"
    * https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01462.html
  Presumably such messages would need to be internationalized.  Many
  other messages are really internal only, and would be a burden to
  impose on our translators.
* I want to support additional destinations for any/all dump messages
  beyond just the dump file and -fopt-info:
  * putting them through the diagnostics subsystem (as "remarks")
  * storing them to disk in a machine-readable format, so that
    3rd-party tools can present useful visualizations of how the
    user's code is being optimized, e.g. prioritized by hotness
* I want to preserve the existing API and most existing uses of it
  (to minimize churn and thus keep our lives easier)

The patches I've been posting so far add additional dump destinations,
like this:

  dump_* (MSG_*) >---> dumpfile.c --+--> dump_file
                                    |
                                    +--> alt_dump_file
                                    |
                                    +--> diagnostic "remarks"
                                    |
                                    +--> "optimization records"
                                         (saved to disk)

I believe you favor an approach of "MSG_NOTE is internal, whereas
MSG_MISSED_OPTIMIZATION and MSG_OPTIMIZED_LOCATIONS are for end users".

I think the "should this dump message be internationalized?" issue
introduces a natural separation between dump messages aimed at
end-users vs purely "internal" dumps; this suggests to me that
we need a new API for such dump messages, designed to support i18n,
and marked as such for gettext, so I favor something like this:

  SOURCES                                DESTINATIONS
  dump_* (MSG_*) >---> dumpfile.c --+--> dump_file
                   |                |
  new dump API -->-+                +--> alt_dump_file
                                    |
                                    +--> diagnostic "remarks"
                                    |
                                    +--> "optimization records"
                                         (saved to disk)

(I'm deliberately being vague about what this i18n-enabled dump API
might look like)

That said, I have a version of the patch kit which does just this:

  dump_* (MSG_*) ----> dumpfile.c --+--> dump_file
                                    |
                                    +--> alt_dump_file
                                    |
                                    +--> diagnostic "remarks"

i.e. generalizing the dump destinations (by adding optinfo internally),
but without requiring the JSON support or touching the dump API . Would
that be suitable as a next step?

[...snip...]

Thoughts?

Thanks
Dave
Richard Biener June 29, 2018, 7:09 a.m. | #3
On Thu, Jun 28, 2018 at 4:29 PM David Malcolm <dmalcolm@redhat.com> wrote:
>

> On Thu, 2018-06-28 at 13:29 +0200, Richard Biener wrote:

> > On Tue, Jun 26, 2018 at 3:54 PM David Malcolm <dmalcolm@redhat.com>

> > wrote:

> > >

> > > On Mon, 2018-06-25 at 15:34 +0200, Richard Biener wrote:

> > > > On Wed, Jun 20, 2018 at 6:34 PM David Malcolm <dmalcolm@redhat.co

> > > > m>

> > > > wrote:

> > > > >

> > > > > Here's v3 of the patch (one big patch this time, rather than a

> > > > > kit).

> > > > >

> > > > > Like the v2 patch kit, this patch reuses the existing dump API,

> > > > > rather than inventing its own.

> > > > >

> > > > > Specifically, it uses the dump_* functions in dumpfile.h that

> > > > > don't

> > > > > take a FILE *, the ones that implicitly write to dump_file

> > > > > and/or

> > > > > alt_dump_file.  I needed a name for them, so I've taken to

> > > > > calling

> > > > > them the "structured dump API" (better name ideas welcome).

> > > > >

> > > > > v3 eliminates v2's optinfo_guard class, instead using

> > > > > "dump_*_loc"

> > > > > calls as delimiters when consolidating "dump_*" calls.  There's

> > > > > a

> > > > > new dump_context class which has responsibility for

> > > > > consolidating

> > > > > them into optimization records.

> > > > >

> > > > > The dump_*_loc calls now capture more than just a location_t:

> > > > > they

> > > > > capture the profile_count and the location in GCC's own sources

> > > > > where

> > > > > the dump is being emitted from.

> > > > >

> > > > > This works by introducing a new "dump_location_t" class as the

> > > > > argument of those dump_*_loc calls.  The dump_location_t can

> > > > > be constructed from a gimple * or from an rtx_insn *, so that

> > > > > rather than writing:

> > > > >

> > > > >   dump_printf_loc (MSG_NOTE, gimple_location (stmt),

> > > > >                    "some message: %i", 42);

> > > > >

> > > > > you can write:

> > > > >

> > > > >   dump_printf_loc (MSG_NOTE, stmt,

> > > > >                    "some message: %i", 42);

> > > > >

> > > > > and the dump_location_t constructor will grab the location_t

> > > > > and

> > > > > profile_count of stmt, and the location of the

> > > > > "dump_printf_loc"

> > > > > callsite (and gracefully handle "stmt" being NULL).

> > > > >

> > > > > Earlier versions of the patch captured the location of the

> > > > > dump_*_loc call via preprocessor hacks, or didn't work

> > > > > properly;

> > > > > this version of the patch works more cleanly: internally,

> > > > > dump_location_t is split into two new classes:

> > > > >   * dump_user_location_t: the location_t and profile_count

> > > > > within

> > > > >     the *user's code*, and

> > > > >   * dump_impl_location_t: the __builtin_FILE/LINE/FUNCTION

> > > > > within

> > > > >     the *implementation* code (i.e. GCC or a plugin), captured

> > > > >     "automagically" via default params

> > > > >

> > > > > These classes are sometimes used elsewhere in the code.  For

> > > > > example, "vect_location" becomes a dump_user_location_t

> > > > > (location_t and profile_count), so that in e.g:

> > > > >

> > > > >   vect_location = find_loop_location (loop);

> > > > >

> > > > > it's capturing the location_t and profile_count, and then when

> > > > > it's used here:

> > > > >

> > > > >   dump_printf_loc (MSG_NOTE, vect_location, "foo");

> > > > >

> > > > > the dump_location_t is constructed from the vect_location

> > > > > plus the dump_impl_location_t at that callsite.

> > > > >

> > > > > In contrast, loop-unroll.c's report_unroll's "locus" param

> > > > > becomes a dump_location_t: we're interested in where it was

> > > > > called from, not in the locations of the various dump_*_loc

> > > > > calls

> > > > > within it.

> > > > >

> > > > > Previous versions of the patch captured a gimple *, and needed

> > > > > GTY markers; in this patch, the dump_user_location_t is now

> > > > > just a

> > > > > location_t and a profile_count.

> > > > >

> > > > > The v2 patch added an overload for dump_printf_loc so that you

> > > > > could pass in either a location_t, or the new type; this

> > > > > version

> > > > > of the patch eliminates that: they all now take

> > > > > dump_location_t.

> > > > >

> > > > > Doing so required adding support for rtx_insn *, so that one

> > > > > can

> > > > > write this kind of thing in RTL passes:

> > > > >

> > > > >   dump_printf_loc (MSG_NOTE, insn, "foo");

> > > > >

> > > > > One knock-on effect is that get_loop_location now returns a

> > > > > dump_user_location_t rather than a location_t, so that it has

> > > > > hotness information.

> > > > >

> > > > > Richi: would you like me to split out this location-handling

> > > > > code into a separate patch?  (It's kind of redundant without

> > > > > adding the remarks and optimization records work, but if that's

> > > > > easier I can do it)

> > > >

> > > > I think that would be easier because it doesn't require the JSON

> > > > stuff and so I'll happily approve it.

> > > >

> > > > Thus - trying to review that bits (and sorry for the delay).

> > > >

> > > > +  location_t srcloc = loc.get_location_t ();

> > > > +

> > > >    if (dump_file && (dump_kind & pflags))

> > > >      {

> > > > -      dump_loc (dump_kind, dump_file, loc);

> > > > +      dump_loc (dump_kind, dump_file, srcloc);

> > > >        print_gimple_stmt (dump_file, gs, spc, dump_flags |

> > > > extra_dump_flags);

> > > >      }

> > > >

> > > >    if (alt_dump_file && (dump_kind & alt_flags))

> > > >      {

> > > > -      dump_loc (dump_kind, alt_dump_file, loc);

> > > > +      dump_loc (dump_kind, alt_dump_file, srcloc);

> > > >        print_gimple_stmt (alt_dump_file, gs, spc, dump_flags |

> > > > extra_dump_flags);

> > > >      }

> > > > +

> > > > +  if (optinfo_enabled_p ())

> > > > +    {

> > > > +      optinfo &info = begin_next_optinfo (loc);

> > > > +      info.handle_dump_file_kind (dump_kind);

> > > > +      info.add_stmt (gs, extra_dump_flags);

> > > > +    }

> > > >

> > > > seeing this in multiple places.  I seem to remember that

> > > > dump_file / alt_dump_file was suposed to handle dumping

> > > > into two locations - a dump file and optinfo (or stdout).  This

> > > > looks

> > > > like the optinfo "stream" is even more separate.  Could that

> > > > obsolete the alt_dump_file stream?  I'd need to review existing

> > > > stuff

> > > > in more detail to answer but maybe you already know from recently

> > > > digging into this.

> > >

> > > Possibly.  I attempted this in v1 of the patch, but it was mixed in

> > > with

> > > a bunch of other stuff.  I'll have another go at doing this.

> > >

> > > > Oh, and all the if (optinfo_enable_p ()) stuff is for the

> > > > followup

> > > > then, right?

> > >

> > > Yes.

> > >

> > > > I like the boiler-plate changes to dump_* using stuff a lot, so

> > > > the

> > > > infrastructure to do that (the location wrapping) and these

> > > > boiler-

> > > > plate

> > > > changes are pre-approved if split out.

> > >

> > > Thanks.  I split out the location wrapping, and have committed it

> > > to

> > > trunk (r262149).  It's not clear to me exactly what other parts you

> > > liked,

> > > so I'm going to try to split out more of the non-JSON bits in the

> > > hope that some parts are good enough as-is, and I'll post them for

> > > review

> > > as followups.

> > >

> > > For reference, here's what I've committed (I added some obvious

> > > changes

> > > to doc/optinfo.texi).

> > >

> > > > I think the *_REMARK stuff should get attention of the respective

> > > > maintainers - not sure what the difference between NOTE and

> > > > REMARK

> > > > is ;)

> > >

> > > Me neither :)  I think it might come down to "this is purely a

> > > debugging

> > > message for a pass maintainer" (MSG_NOTE) vs "this is a high-level

> > > thing

> > > that an advanced user want to see" (MSG_REMARK???).

> > >

> > > One idea that occurred to me: are we overusing dump_flags_t?  It's

> > > a

> > > mixture of TDF_* bitfields (used by our internal dumps) plus MSG_*

> > > bitfields (used with -fopt-info).  IIRC the only TDF_ bitfield

> > > that's

> > > ever used with the MSG_* bitfields is TDF_DETAILS.  Might it make

> > > sense

> > > to split out the MSG_* bitfields into a different type

> > > (dump_level_t???)

> > > to reinforce this split?  (and thus the dump_* entrypoints that

> > > don't take

> > > a FILE * would take this new type).  I'm not sure if this is a good

> > > idea

> > > or not.

> >

> > Making it even more complicated doesn't make it easier to use it

> > "correctly".  So I'd rather try to simplify it.  How passes use

> > TDF_DETAILS vs. non-details is already highly inconsistent.  This

> > is why I liked the original optinfo work because it somehow made

> > user-interesting vs. developer-interesting with the same API

> > (and to the same dump-file).  Just that MSG_NOTE is exposed to

> > users while I think it should be developer-only...

> >

> > IMHO the TDF_ stuff should control things at the stmt/tree/BB level,

> > thus be IL specific flags while the MSG_ stuff should control

> > pass-specific dumping detail.

>

> You mention user-interesting vs developer-interesting, and whether it

> would be the same API and/or the same dump-file.

>

> I've posted a bunch of patches here, some small, some large, trying

> various different appoaches, coming at the problem from at least

> two directions, so maybe it's worth discussing the overall direction

> here (with some ASCII art!)

>

> * what is the status quo?

> * what do we want to achieve?

> * how do we get there?

>

> Status quo:

> * We have the "dump_*(MSG_*)" API which hides the destination of

>   where we're dumping to (currently dump_file and/or alt_file_file,

>   conditionalized via flags).

>

>   dump_* (MSG_*) ----> dumpfile.c --+--> dump_file

>                                     |

>                                     `--> alt_dump_file

>

> * As of r262149 (the dump_location_t commit) dumpfile.c receives the

>   hotness and emission location of each dump_*_loc call, but currently

>   it does nothing with that information.

> * The messages that we emit through the "dump_*(MSG_*)" API and their

>   "levels" are currently rather haphazard.  Some are close to being

>   suitable for end-users, but most seem never intended to be

>   end-user-facing.  For reference:

>     grep -nH -e MSG_MISSED_OPTIMIZATION *.c *.cc | wc -l

>     452

>     grep -nH -e MSG_NOTE *.c *.cc | wc -l

>     551

>     grep -nH -e MSG_OPTIMIZED_LOCATIONS *.c *.cc | wc -l

>     39

>   (though some of these are support code e.g. in dumpfile.c, rather

>   than uses).


Yep.  I believe MSG_NOTE was the fallout of optinfo introduction and my
request to share the same API with dumping to the dumpfile.  MSG_NOTE
just received "anything else" ...

> * The API builds up messages programatically, which is hostile to

>   i18n.  The API isn't marked as needing i18n for its strings (which

>   IIRC is via using "gmsgid" as a param name being special-cased in

>   the gettext toolchain).

>

> What do we want to achieve?

> * I want end-users to be able to enable high-level dump messages about

>   optimizations, and for those messages to be decipherable without

>   reading GCC sources e.g. the opt_problem idea posted here:

>   * "[PATCH] [RFC] Higher-level reporting of vectorization problems"

>     * https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01462.html

>   Presumably such messages would need to be internationalized.  Many

>   other messages are really internal only, and would be a burden to

>   impose on our translators.

> * I want to support additional destinations for any/all dump messages

>   beyond just the dump file and -fopt-info:

>   * putting them through the diagnostics subsystem (as "remarks")

>   * storing them to disk in a machine-readable format, so that

>     3rd-party tools can present useful visualizations of how the

>     user's code is being optimized, e.g. prioritized by hotness

> * I want to preserve the existing API and most existing uses of it

>   (to minimize churn and thus keep our lives easier)

>

> The patches I've been posting so far add additional dump destinations,

> like this:

>

>   dump_* (MSG_*) >---> dumpfile.c --+--> dump_file

>                                     |

>                                     +--> alt_dump_file

>                                     |

>                                     +--> diagnostic "remarks"

>                                     |

>                                     +--> "optimization records"

>                                          (saved to disk)

>

> I believe you favor an approach of "MSG_NOTE is internal, whereas

> MSG_MISSED_OPTIMIZATION and MSG_OPTIMIZED_LOCATIONS are for end users".


Yeah, kind of.  You introduced another MSG_ variant and in the end we might
need to go that way.  I guess renaming MSG_NOTE to MSG_DEBUG would
be a step in the direction to reflect reality ;)

> I think the "should this dump message be internationalized?" issue

> introduces a natural separation between dump messages aimed at

> end-users vs purely "internal" dumps; this suggests to me that

> we need a new API for such dump messages, designed to support i18n,

> and marked as such for gettext, so I favor something like this:

>

>   SOURCES                                DESTINATIONS

>   dump_* (MSG_*) >---> dumpfile.c --+--> dump_file

>                    |                |

>   new dump API -->-+                +--> alt_dump_file

>                                     |

>                                     +--> diagnostic "remarks"

>                                     |

>                                     +--> "optimization records"

>                                          (saved to disk)

>

> (I'm deliberately being vague about what this i18n-enabled dump API

> might look like)


I know I threw i18n into the discussion - but I'd say we should focus
on other details first.  I think there's no need to _require_ the "debug"
dumpfile stuff not be translated, but to make it easier for translators
I suppose being able to "amend" the API calls with a "do not translate"
variant would be OK.

So in what way is the dump_* API not suitable for translation
that the diagnostic machinery (warning/error) does not suffer from?

That is, if we internally make dump_* go through the pretty-printers
we can handle stuff like quoting or even stmts/expressions.  I think
that would be useful for dumping to dumpfiles as well.

>

> That said, I have a version of the patch kit which does just this:

>

>   dump_* (MSG_*) ----> dumpfile.c --+--> dump_file

>                                     |

>                                     +--> alt_dump_file

>                                     |

>                                     +--> diagnostic "remarks"

>

> i.e. generalizing the dump destinations (by adding optinfo internally),

> but without requiring the JSON support or touching the dump API . Would

> that be suitable as a next step?


Yes.  Thanks for going step-by-step btw, this is really useful in simplifying
the picture.

Richard.

>

> [...snip...]

>

> Thoughts?

>

> Thanks

> Dave

>

Patch

diff --git a/gcc/cfgloop.c b/gcc/cfgloop.c
index 8af793c..e27cd39 100644
--- a/gcc/cfgloop.c
+++ b/gcc/cfgloop.c
@@ -1800,7 +1800,7 @@  loop_exits_from_bb_p (struct loop *loop, basic_block bb)
 
 /* Return location corresponding to the loop control condition if possible.  */
 
-location_t
+dump_user_location_t
 get_loop_location (struct loop *loop)
 {
   rtx_insn *insn = NULL;
@@ -1819,7 +1819,7 @@  get_loop_location (struct loop *loop)
       FOR_BB_INSNS_REVERSE (desc->in_edge->src, insn)
         {
           if (INSN_P (insn) && INSN_HAS_LOCATION (insn))
-            return INSN_LOCATION (insn);
+            return insn;
         }
     }
   /* If loop has a single exit, then the loop control branch
@@ -1829,24 +1829,24 @@  get_loop_location (struct loop *loop)
       FOR_BB_INSNS_REVERSE (exit->src, insn)
         {
           if (INSN_P (insn) && INSN_HAS_LOCATION (insn))
-            return INSN_LOCATION (insn);
+            return insn;
         }
     }
   /* Next check the latch, to see if it is non-empty.  */
   FOR_BB_INSNS_REVERSE (loop->latch, insn)
     {
       if (INSN_P (insn) && INSN_HAS_LOCATION (insn))
-        return INSN_LOCATION (insn);
+        return insn;
     }
   /* Finally, if none of the above identifies the loop control branch,
      return the first location in the loop header.  */
   FOR_BB_INSNS (loop->header, insn)
     {
       if (INSN_P (insn) && INSN_HAS_LOCATION (insn))
-        return INSN_LOCATION (insn);
+        return insn;
     }
   /* If all else fails, simply return the current function location.  */
-  return DECL_SOURCE_LOCATION (current_function_decl);
+  return dump_user_location_t::from_function_decl (current_function_decl);
 }
 
 /* Records that every statement in LOOP is executed I_BOUND times.
diff --git a/gcc/cfgloop.h b/gcc/cfgloop.h
index af9bfab..80a31c4 100644
--- a/gcc/cfgloop.h
+++ b/gcc/cfgloop.h
@@ -357,7 +357,7 @@  extern bool loop_exit_edge_p (const struct loop *, const_edge);
 extern bool loop_exits_to_bb_p (struct loop *, basic_block);
 extern bool loop_exits_from_bb_p (struct loop *, basic_block);
 extern void mark_loop_exit_edges (void);
-extern location_t get_loop_location (struct loop *loop);
+extern dump_user_location_t get_loop_location (struct loop *loop);
 
 /* Loops & cfg manipulation.  */
 extern basic_block *get_loop_body (const struct loop *);
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 04b6919..7cfb8a0 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -928,8 +928,7 @@  walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets,
 	    }
           if (dump_enabled_p ())
             {
-	      location_t locus = gimple_location_safe (edge->call_stmt);
-	      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, locus,
+	      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, edge->call_stmt,
 			       "devirtualizing call in %s to %s\n",
 			       edge->caller->name (), target->name ());
 	    }
diff --git a/gcc/coverage.c b/gcc/coverage.c
index 84fff13..350cc45 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -342,12 +342,16 @@  get_coverage_counts (unsigned counter, unsigned expected,
       static int warned = 0;
 
       if (!warned++ && dump_enabled_p ())
-	dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, input_location,
-                         (flag_guess_branch_prob
-                          ? "file %s not found, execution counts estimated\n"
-                          : "file %s not found, execution counts assumed to "
-                            "be zero\n"),
-                         da_file_name);
+	{
+	  dump_user_location_t loc
+	    = dump_user_location_t::from_location_t (input_location);
+	  dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
+			   (flag_guess_branch_prob
+			    ? "file %s not found, execution counts estimated\n"
+			    : "file %s not found, execution counts assumed to "
+			    "be zero\n"),
+			   da_file_name);
+	}
       return NULL;
     }
   if (PARAM_VALUE (PARAM_PROFILE_FUNC_INTERNAL_ID))
@@ -378,7 +382,9 @@  get_coverage_counts (unsigned counter, unsigned expected,
 		    "its profile data (counter %qs)", id, ctr_names[counter]);
       if (warning_printed && dump_enabled_p ())
 	{
-          dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, input_location,
+	  dump_user_location_t loc
+	    = dump_user_location_t::from_location_t (input_location);
+          dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
                            "use -Wno-error=coverage-mismatch to tolerate "
                            "the mismatch but performance may drop if the "
                            "function is hot\n");
@@ -386,7 +392,7 @@  get_coverage_counts (unsigned counter, unsigned expected,
 	  if (!seen_error ()
 	      && !warned++)
 	    {
-	      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, input_location,
+	      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
                                "coverage mismatch ignored\n");
 	      dump_printf (MSG_OPTIMIZED_LOCATIONS,
                            flag_guess_branch_prob
diff --git a/gcc/doc/optinfo.texi b/gcc/doc/optinfo.texi
index 8c28501..6202802 100644
--- a/gcc/doc/optinfo.texi
+++ b/gcc/doc/optinfo.texi
@@ -168,7 +168,7 @@  when any of the following flags is enabled
 
 @example
 int report_flags = MSG_OPTIMIZED_LOCATIONS | TDF_RTL | TDF_DETAILS;
-dump_printf_loc (report_flags, locus,
+dump_printf_loc (report_flags, insn,
                  "loop turned into non-loop; it never loops.\n");
 @end example
 
@@ -181,7 +181,19 @@  Output gimple statement.
 
 Note that the above methods also have variants prefixed with
 @code{_loc}, such as @code{dump_printf_loc}, which are similar except
-they also output the source location information.
+they also output the source location information.  The @code{_loc} variants
+take a @code{const dump_location_t &}.  This class can be constructed from
+a @code{gimple *} or from a @code{rtx_insn *}, and so callers can pass
+a @code{gimple *} or a @code{rtx_insn *} as the @code{_loc} argument.
+The @code{dump_location_t} constructor will extract the source location
+from the statement or instruction, along with the profile count, and
+the location in GCC's own source code (or the plugin) from which the dump
+call was emitted.  Only the source location is currently used.
+There is also a @code{dump_user_location_t} class, capturing the
+source location and profile count, but not the dump emission location,
+so that locations in the user's code can be passed around.  This
+can also be constructed from a @code{gimple *} and from a @code{rtx_insn *},
+and it too can be passed as the @code{_loc} argument.
 
 @end ftable
 
diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 2f11284..122e420 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -29,6 +29,10 @@  along with GCC; see the file COPYING3.  If not see
 #include "profile-count.h"
 #include "tree-cfg.h"
 #include "langhooks.h"
+#include "backend.h" /* for gimple.h.  */
+#include "gimple.h" /* for dump_user_location_t ctor.  */
+#include "rtl.h" /* for dump_user_location_t ctor.  */
+#include "selftest.h"
 
 /* If non-NULL, return one past-the-end of the matching SUBPART of
    the WHOLE string.  */
@@ -358,9 +362,51 @@  dump_open_alternate_stream (struct dump_file_info *dfi)
   return stream;
 }
 
+/* Construct a dump_user_location_t from STMT (using its location and
+   hotness).  */
+
+dump_user_location_t::dump_user_location_t (gimple *stmt)
+: m_count (), m_loc (UNKNOWN_LOCATION)
+{
+  if (stmt)
+    {
+      if (stmt->bb)
+	m_count = stmt->bb->count;
+      m_loc = gimple_location (stmt);
+    }
+}
+
+/* Construct a dump_user_location_t from an RTL instruction (using its
+   location and hotness).  */
+
+dump_user_location_t::dump_user_location_t (rtx_insn *insn)
+: m_count (), m_loc (UNKNOWN_LOCATION)
+{
+  if (insn)
+    {
+      basic_block bb = BLOCK_FOR_INSN (insn);
+      if (bb)
+	m_count = bb->count;
+      m_loc = INSN_LOCATION (insn);
+    }
+}
+
+/* Construct from a function declaration.  This one requires spelling out
+   to avoid accidentally constructing from other kinds of tree.  */
+
+dump_user_location_t
+dump_user_location_t::from_function_decl (tree fndecl)
+{
+  gcc_assert (fndecl);
+
+  // FIXME: profile count for function?
+  return dump_user_location_t (profile_count (),
+			       DECL_SOURCE_LOCATION (fndecl));
+}
+
 /* Print source location on DFILE if enabled.  */
 
-void
+static void
 dump_loc (dump_flags_t dump_kind, FILE *dfile, source_location loc)
 {
   if (dump_kind)
@@ -393,18 +439,19 @@  dump_gimple_stmt (dump_flags_t dump_kind, dump_flags_t extra_dump_flags,
 /* Similar to dump_gimple_stmt, except additionally print source location.  */
 
 void
-dump_gimple_stmt_loc (dump_flags_t dump_kind, source_location loc,
+dump_gimple_stmt_loc (dump_flags_t dump_kind, const dump_location_t &loc,
 		      dump_flags_t extra_dump_flags, gimple *gs, int spc)
 {
+  location_t srcloc = loc.get_location_t ();
   if (dump_file && (dump_kind & pflags))
     {
-      dump_loc (dump_kind, dump_file, loc);
+      dump_loc (dump_kind, dump_file, srcloc);
       print_gimple_stmt (dump_file, gs, spc, dump_flags | extra_dump_flags);
     }
 
   if (alt_dump_file && (dump_kind & alt_flags))
     {
-      dump_loc (dump_kind, alt_dump_file, loc);
+      dump_loc (dump_kind, alt_dump_file, srcloc);
       print_gimple_stmt (alt_dump_file, gs, spc, dump_flags | extra_dump_flags);
     }
 }
@@ -423,27 +470,6 @@  dump_generic_expr (dump_flags_t dump_kind, dump_flags_t extra_dump_flags,
       print_generic_expr (alt_dump_file, t, dump_flags | extra_dump_flags);
 }
 
-
-/* Similar to dump_generic_expr, except additionally print the source
-   location.  */
-
-void
-dump_generic_expr_loc (dump_flags_t dump_kind, source_location loc,
-		       dump_flags_t extra_dump_flags, tree t)
-{
-  if (dump_file && (dump_kind & pflags))
-    {
-      dump_loc (dump_kind, dump_file, loc);
-      print_generic_expr (dump_file, t, dump_flags | extra_dump_flags);
-    }
-
-  if (alt_dump_file && (dump_kind & alt_flags))
-    {
-      dump_loc (dump_kind, alt_dump_file, loc);
-      print_generic_expr (alt_dump_file, t, dump_flags | extra_dump_flags);
-    }
-}
-
 /* Output a formatted message using FORMAT on appropriate dump streams.  */
 
 void
@@ -469,13 +495,14 @@  dump_printf (dump_flags_t dump_kind, const char *format, ...)
 /* Similar to dump_printf, except source location is also printed.  */
 
 void
-dump_printf_loc (dump_flags_t dump_kind, source_location loc,
+dump_printf_loc (dump_flags_t dump_kind, const dump_location_t &loc,
 		 const char *format, ...)
 {
+  location_t srcloc = loc.get_location_t ();
   if (dump_file && (dump_kind & pflags))
     {
       va_list ap;
-      dump_loc (dump_kind, dump_file, loc);
+      dump_loc (dump_kind, dump_file, srcloc);
       va_start (ap, format);
       vfprintf (dump_file, format, ap);
       va_end (ap);
@@ -484,7 +511,7 @@  dump_printf_loc (dump_flags_t dump_kind, source_location loc,
   if (alt_dump_file && (dump_kind & alt_flags))
     {
       va_list ap;
-      dump_loc (dump_kind, alt_dump_file, loc);
+      dump_loc (dump_kind, alt_dump_file, srcloc);
       va_start (ap, format);
       vfprintf (alt_dump_file, format, ap);
       va_end (ap);
@@ -1059,3 +1086,53 @@  enable_rtl_dump_file (void)
 			    NULL);
   return num_enabled > 0;
 }
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Verify that the dump_location_t constructors capture the source location
+   at which they were called (provided that the build compiler is sufficiently
+   recent).  */
+
+static void
+test_impl_location ()
+{
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)
+  /* Default ctor.  */
+  {
+    dump_location_t loc;
+    const int expected_line = __LINE__ - 1;
+    ASSERT_STR_CONTAINS (loc.get_impl_location ().m_file, "dumpfile.c");
+    ASSERT_EQ (loc.get_impl_location ().m_line, expected_line);
+  }
+
+  /* Constructing from a gimple.  */
+  {
+    dump_location_t loc ((gimple *)NULL);
+    const int expected_line = __LINE__ - 1;
+    ASSERT_STR_CONTAINS (loc.get_impl_location ().m_file, "dumpfile.c");
+    ASSERT_EQ (loc.get_impl_location ().m_line, expected_line);
+  }
+
+  /* Constructing from an rtx_insn.  */
+  {
+    dump_location_t loc ((rtx_insn *)NULL);
+    const int expected_line = __LINE__ - 1;
+    ASSERT_STR_CONTAINS (loc.get_impl_location ().m_file, "dumpfile.c");
+    ASSERT_EQ (loc.get_impl_location ().m_line, expected_line);
+  }
+#endif
+}
+
+/* Run all of the selftests within this file.  */
+
+void
+dumpfile_c_tests ()
+{
+  test_impl_location ();
+}
+
+} // namespace selftest
+
+#endif /* CHECKING_P */
diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
index f6ad670..90d8930 100644
--- a/gcc/dumpfile.h
+++ b/gcc/dumpfile.h
@@ -21,6 +21,7 @@  along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_DUMPFILE_H
 #define GCC_DUMPFILE_H 1
 
+#include "profile-count.h"
 
 /* Different tree dump places.  When you add new tree dump places,
    extend the DUMP_FILES array in dumpfile.c.  */
@@ -268,20 +269,165 @@  struct dump_file_info
   bool graph_dump_initialized;
 };
 
+/* A class for describing where in the user's source that a dump message
+   relates to, with various constructors for convenience.
+   In particular, this lets us associate dump messages
+   with hotness information (e.g. from PGO), allowing them to
+   be prioritized by code hotness.  */
+
+class dump_user_location_t
+{
+ public:
+  /* Default constructor, analogous to UNKNOWN_LOCATION.  */
+  dump_user_location_t () : m_count (), m_loc (UNKNOWN_LOCATION) {}
+
+  /* Construct from a gimple statement (using its location and hotness).  */
+  dump_user_location_t (gimple *stmt);
+
+  /* Construct from an RTL instruction (using its location and hotness).  */
+  dump_user_location_t (rtx_insn *insn);
+
+  /* Construct from a location_t.  This one is deprecated (since it doesn't
+     capture hotness information); it thus needs to be spelled out.  */
+  static dump_user_location_t
+  from_location_t (location_t loc)
+  {
+    return dump_user_location_t (profile_count (), loc);
+  }
+
+  /* Construct from a function declaration.  This one requires spelling out
+     to avoid accidentally constructing from other kinds of tree.  */
+  static dump_user_location_t
+  from_function_decl (tree fndecl);
+
+  profile_count get_count () const { return m_count; }
+  location_t get_location_t () const { return m_loc; }
+
+ private:
+  /* Private ctor from count and location, for use by from_location_t.  */
+  dump_user_location_t (profile_count count, location_t loc)
+    : m_count (count), m_loc (loc)
+  {}
+
+  profile_count m_count;
+  location_t m_loc;
+};
+
+/* A class for identifying where in the compiler's own source
+   (or a plugin) that a dump message is being emitted from.  */
+
+struct dump_impl_location_t
+{
+  dump_impl_location_t (
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)
+			const char *file = __builtin_FILE (),
+			int line = __builtin_LINE (),
+			const char *function = __builtin_FUNCTION ()
+#else
+			const char *file = __FILE__,
+			int line = __LINE__,
+			const char *function = NULL
+#endif
+  )
+  : m_file (file), m_line (line), m_function (function)
+  {}
+
+  const char *m_file;
+  int m_line;
+  const char *m_function;
+};
+
+/* A bundle of information for describing the location of a dump message:
+   (a) the source location and hotness within the user's code, together with
+   (b) the source location within the compiler/plugin.
+
+   The constructors use default parameters so that (b) gets sets up
+   automatically.
+
+   The upshot is that you can pass in e.g. a gimple * to dump_printf_loc,
+   and the dump call will automatically record where in GCC's source
+   code the dump was emitted from.  */
+
+class dump_location_t
+{
+ public:
+  /* Default constructor, analogous to UNKNOWN_LOCATION.  */
+  dump_location_t (const dump_impl_location_t &impl_location
+		     = dump_impl_location_t ())
+  : m_user_location (dump_user_location_t ()),
+    m_impl_location (impl_location)
+  {
+  }
+
+  /* Construct from a gimple statement (using its location and hotness).  */
+  dump_location_t (gimple *stmt,
+		   const dump_impl_location_t &impl_location
+		     = dump_impl_location_t ())
+  : m_user_location (dump_user_location_t (stmt)),
+    m_impl_location (impl_location)
+  {
+  }
+
+  /* Construct from an RTL instruction (using its location and hotness).  */
+  dump_location_t (rtx_insn *insn,
+		   const dump_impl_location_t &impl_location
+		   = dump_impl_location_t ())
+  : m_user_location (dump_user_location_t (insn)),
+    m_impl_location (impl_location)
+  {
+  }
+
+  /* Construct from a dump_user_location_t.  */
+  dump_location_t (const dump_user_location_t &user_location,
+		   const dump_impl_location_t &impl_location
+		     = dump_impl_location_t ())
+  : m_user_location (user_location),
+    m_impl_location (impl_location)
+  {
+  }
+
+  /* Construct from a location_t.  This one is deprecated (since it doesn't
+     capture hotness information), and thus requires spelling out.  */
+  static dump_location_t
+  from_location_t (location_t loc,
+		   const dump_impl_location_t &impl_location
+		     = dump_impl_location_t ())
+  {
+    return dump_location_t (dump_user_location_t::from_location_t (loc),
+			    impl_location);
+  }
+
+  const dump_user_location_t &
+  get_user_location () const { return m_user_location; }
+
+  const dump_impl_location_t &
+  get_impl_location () const { return m_impl_location; }
+
+  location_t get_location_t () const
+  {
+    return m_user_location.get_location_t ();
+  }
+
+  profile_count get_count () const { return m_user_location.get_count (); }
+
+ private:
+  dump_user_location_t m_user_location;
+  dump_impl_location_t m_impl_location;
+};
+
 /* In dumpfile.c */
 extern FILE *dump_begin (int, dump_flags_t *);
 extern void dump_end (int, FILE *);
 extern int opt_info_switch_p (const char *);
 extern const char *dump_flag_name (int);
 extern void dump_printf (dump_flags_t, const char *, ...) ATTRIBUTE_PRINTF_2;
-extern void dump_printf_loc (dump_flags_t, source_location,
-                             const char *, ...) ATTRIBUTE_PRINTF_3;
+extern void dump_printf_loc (dump_flags_t, const dump_location_t &,
+			     const char *, ...) ATTRIBUTE_PRINTF_3;
 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, source_location, dump_flags_t, tree);
 extern void dump_generic_expr (dump_flags_t, dump_flags_t, tree);
-extern void dump_gimple_stmt_loc (dump_flags_t, source_location, dump_flags_t,
-				  gimple *, int);
+extern void dump_gimple_stmt_loc (dump_flags_t, const dump_location_t &,
+				  dump_flags_t, gimple *, int);
 extern void dump_gimple_stmt (dump_flags_t, dump_flags_t, gimple *, int);
 extern void print_combine_total_stats (void);
 extern bool enable_rtl_dump_file (void);
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index a01bce7..f12e4a7 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -347,8 +347,7 @@  fold_gimple_assign (gimple_stmt_iterator *si)
 		  {
 		    if (dump_enabled_p ())
 		      {
-			location_t loc = gimple_location_safe (stmt);
-			dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
+			dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt,
 					 "resolving virtual function address "
 					 "reference to function %s\n",
 					 targets.length () == 1
@@ -4061,8 +4060,7 @@  gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace)
 	      tree lhs = gimple_call_lhs (stmt);
 	      if (dump_enabled_p ())
 		{
-		  location_t loc = gimple_location_safe (stmt);
-		  dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
+		  dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt,
 				   "folding virtual function call to %s\n",
 		 		   targets.length () == 1
 		  		   ? targets[0]->name ()
diff --git a/gcc/gimple-loop-interchange.cc b/gcc/gimple-loop-interchange.cc
index eb35263..08aeb8e 100644
--- a/gcc/gimple-loop-interchange.cc
+++ b/gcc/gimple-loop-interchange.cc
@@ -523,7 +523,7 @@  loop_cand::analyze_iloop_reduction_var (tree var)
 
   /* Handle and verify a series of stmts feeding the reduction op.  */
   if (single_use != next_def
-      && !check_reduction_path (UNKNOWN_LOCATION, m_loop, phi, next,
+      && !check_reduction_path (dump_user_location_t (), m_loop, phi, next,
 				gimple_assign_rhs_code (single_use)))
     return false;
 
@@ -1578,7 +1578,7 @@  bool
 tree_loop_interchange::interchange (vec<data_reference_p> datarefs,
 				    vec<ddr_p> ddrs)
 {
-  location_t loc = find_loop_location (m_loop_nest[0]);
+  dump_user_location_t loc = find_loop_location (m_loop_nest[0]);
   bool changed_p = false;
   /* In each iteration we try to interchange I-th loop with (I+1)-th loop.
      The overall effect is to push inner loop to outermost level in whole
diff --git a/gcc/graphite-isl-ast-to-gimple.c b/gcc/graphite-isl-ast-to-gimple.c
index b607b12..9e78465 100644
--- a/gcc/graphite-isl-ast-to-gimple.c
+++ b/gcc/graphite-isl-ast-to-gimple.c
@@ -1409,7 +1409,7 @@  scop_to_isl_ast (scop_p scop)
   isl_ctx_set_max_operations (scop->isl_context, old_max_operations);
   if (isl_ctx_last_error (scop->isl_context) != isl_error_none)
     {
-      location_t loc = find_loop_location
+      dump_user_location_t loc = find_loop_location
 	(scop->scop_info->region.entry->dest->loop_father);
       if (isl_ctx_last_error (scop->isl_context) == isl_error_quota)
 	dump_printf_loc (MSG_MISSED_OPTIMIZATION, loc,
@@ -1518,7 +1518,7 @@  graphite_regenerate_ast_isl (scop_p scop)
 
   if (t.codegen_error_p ())
     {
-      location_t loc = find_loop_location
+      dump_user_location_t loc = find_loop_location
 	(scop->scop_info->region.entry->dest->loop_father);
       dump_printf_loc (MSG_MISSED_OPTIMIZATION, loc,
 		       "loop nest not optimized, code generation error\n");
diff --git a/gcc/graphite-optimize-isl.c b/gcc/graphite-optimize-isl.c
index 456a797..35e9ac0 100644
--- a/gcc/graphite-optimize-isl.c
+++ b/gcc/graphite-optimize-isl.c
@@ -160,7 +160,7 @@  optimize_isl (scop_p scop)
   if (!scop->transformed_schedule
       || isl_ctx_last_error (scop->isl_context) != isl_error_none)
     {
-      location_t loc = find_loop_location
+      dump_user_location_t loc = find_loop_location
 	(scop->scop_info->region.entry->dest->loop_father);
       if (isl_ctx_last_error (scop->isl_context) == isl_error_quota)
 	dump_printf_loc (MSG_MISSED_OPTIMIZATION, loc,
@@ -182,7 +182,7 @@  optimize_isl (scop_p scop)
 
   if (same_schedule)
     {
-      location_t loc = find_loop_location
+      dump_user_location_t loc = find_loop_location
 	(scop->scop_info->region.entry->dest->loop_father);
       dump_printf_loc (MSG_NOTE, loc,
 		       "loop nest not optimized, optimized schedule is "
diff --git a/gcc/graphite.c b/gcc/graphite.c
index bcf4828..ddf16a8 100644
--- a/gcc/graphite.c
+++ b/gcc/graphite.c
@@ -412,7 +412,7 @@  graphite_transform_loops (void)
 	changed = true;
 	if (graphite_regenerate_ast_isl (scop))
 	  {
-	    location_t loc = find_loop_location
+	    dump_user_location_t loc = find_loop_location
 	      (scops[i]->scop_info->region.entry->dest->loop_father);
 	    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
 			     "loop nest optimized\n");
diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 308b6e6..e99d8cc 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -3755,8 +3755,7 @@  ipa_devirt (void)
 	      {
 		if (dump_enabled_p ())
                   {
-                    location_t locus = gimple_location_safe (e->call_stmt);
-                    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, locus,
+                    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, e->call_stmt,
 				     "speculatively devirtualizing call "
 				     "in %s to %s\n",
 				     n->dump_name (),
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 000c05f..8b19fe3 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -2842,8 +2842,7 @@  ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target,
 	    {
 	      if (dump_enabled_p ())
 		{
-		  location_t loc = gimple_location_safe (ie->call_stmt);
-		  dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
+		  dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, ie->call_stmt,
 				   "discovered direct call non-invariant %s\n",
 				   ie->caller->dump_name ());
 		}
@@ -2853,8 +2852,7 @@  ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target,
 
           if (dump_enabled_p ())
 	    {
-	      location_t loc = gimple_location_safe (ie->call_stmt);
-	      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
+	      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, ie->call_stmt,
 			       "discovered direct call to non-function in %s, "
 			       "making it __builtin_unreachable\n",
 			       ie->caller->dump_name ());
@@ -2942,9 +2940,7 @@  ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target,
      }
   if (dump_enabled_p ())
     {
-      location_t loc = gimple_location_safe (ie->call_stmt);
-
-      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
+      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, ie->call_stmt,
 		       "converting indirect call in %s to direct call to %s\n",
 		       ie->caller->name (), callee->name ());
     }
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 82fc334..3b6b5e5 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -225,13 +225,8 @@  walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets,
 		       (builtin_decl_implicit (BUILT_IN_UNREACHABLE));
 
 	  if (dump_enabled_p ())
-            {
-	      location_t locus;
-	      if (edge->call_stmt)
-		locus = gimple_location (edge->call_stmt);
-	      else
-		locus = UNKNOWN_LOCATION;
-	      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, locus,
+	    {
+	      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, edge->call_stmt,
 			       "devirtualizing call in %s to %s\n",
 			       edge->caller->dump_name (),
 			       target->dump_name ());
diff --git a/gcc/loop-unroll.c b/gcc/loop-unroll.c
index 5a03932..48bbda0 100644
--- a/gcc/loop-unroll.c
+++ b/gcc/loop-unroll.c
@@ -189,7 +189,7 @@  static rtx get_expansion (struct var_to_expand *);
    appropriate given the dump or -fopt-info settings.  */
 
 static void
-report_unroll (struct loop *loop, location_t locus)
+report_unroll (struct loop *loop, dump_location_t locus)
 {
   dump_flags_t report_flags = MSG_OPTIMIZED_LOCATIONS | TDF_DETAILS;
 
@@ -220,7 +220,7 @@  decide_unrolling (int flags)
   FOR_EACH_LOOP (loop, LI_FROM_INNERMOST)
     {
       loop->lpt_decision.decision = LPT_NONE;
-      location_t locus = get_loop_location (loop);
+      dump_user_location_t locus = get_loop_location (loop);
 
       if (dump_enabled_p ())
 	dump_printf_loc (MSG_NOTE, locus,
diff --git a/gcc/omp-grid.c b/gcc/omp-grid.c
index ffa301e..6edc92f 100644
--- a/gcc/omp-grid.c
+++ b/gcc/omp-grid.c
@@ -91,7 +91,7 @@  struct grid_prop
   bool tiling;
   /* Location of the target construct for optimization information
      messages.  */
-  location_t target_loc;
+  dump_user_location_t target_loc;
   /* The collapse clause of the involved loops.  Collapse value of all of them
      must be the same for gridification to take place.  */
   size_t collapse;
@@ -177,10 +177,10 @@  grid_find_single_omp_among_assignments_1 (gimple_seq seq, grid_prop *grid,
 				   GRID_MISSED_MSG_PREFIX "%s construct "
 				   "contains multiple OpenMP constructs\n",
 				   name);
-		  dump_printf_loc (MSG_NOTE, gimple_location (*ret),
+		  dump_printf_loc (MSG_NOTE, *ret,
 				   "The first OpenMP construct within "
 				   "a parallel\n");
-		  dump_printf_loc (MSG_NOTE, gimple_location (stmt),
+		  dump_printf_loc (MSG_NOTE, stmt,
 				   "The second OpenMP construct within "
 				   "a parallel\n");
 		}
@@ -195,7 +195,7 @@  grid_find_single_omp_among_assignments_1 (gimple_seq seq, grid_prop *grid,
 	      dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,
 			       GRID_MISSED_MSG_PREFIX "%s construct contains "
 			       "a complex statement\n", name);
-	      dump_printf_loc (MSG_NOTE, gimple_location (stmt),
+	      dump_printf_loc (MSG_NOTE, stmt,
 			       "This statement cannot be analyzed for "
 			       "gridification\n");
 	    }
@@ -286,7 +286,7 @@  grid_find_ungridifiable_statement (gimple_stmt_iterator *gsi,
    loop that is evaluated for possible gridification.  */
 
 static bool
-grid_parallel_clauses_gridifiable (gomp_parallel *par, location_t tloc)
+grid_parallel_clauses_gridifiable (gomp_parallel *par, dump_user_location_t tloc)
 {
   tree clauses = gimple_omp_parallel_clauses (par);
   while (clauses)
@@ -300,7 +300,7 @@  grid_parallel_clauses_gridifiable (gomp_parallel *par, location_t tloc)
 			       GRID_MISSED_MSG_PREFIX "because there is "
 			       "a num_threads clause of the parallel "
 			       "construct\n");
-	      dump_printf_loc (MSG_NOTE, gimple_location (par),
+	      dump_printf_loc (MSG_NOTE, par,
 			       "Parallel construct has a num_threads clause\n");
 	    }
 	  return false;
@@ -311,7 +311,7 @@  grid_parallel_clauses_gridifiable (gomp_parallel *par, location_t tloc)
 	      dump_printf_loc (MSG_MISSED_OPTIMIZATION, tloc,
 			       GRID_MISSED_MSG_PREFIX "a reduction clause "
 			       "is present\n ");
-	      dump_printf_loc (MSG_NOTE, gimple_location (par),
+	      dump_printf_loc (MSG_NOTE, par,
 			       "Parallel construct has a reduction clause\n");
 	    }
 	  return false;
@@ -341,7 +341,7 @@  grid_inner_loop_gridifiable_p (gomp_for *gfor, grid_prop *grid)
 			   GRID_MISSED_MSG_PREFIX "the inner loop "
 			   "loop bounds computation contains a complex "
 			   "statement\n");
-	  dump_printf_loc (MSG_NOTE, gimple_location (gfor),
+	  dump_printf_loc (MSG_NOTE, gfor,
 			   "Loop construct cannot be analyzed for "
 			   "gridification\n");
 	}
@@ -361,7 +361,7 @@  grid_inner_loop_gridifiable_p (gomp_for *gfor, grid_prop *grid)
 		  dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,
 				   GRID_MISSED_MSG_PREFIX "the inner loop "
 				   "has a non-automatic schedule clause\n");
-		  dump_printf_loc (MSG_NOTE, gimple_location (gfor),
+		  dump_printf_loc (MSG_NOTE, gfor,
 				   "Loop construct has a non automatic "
 				   "schedule clause\n");
 		}
@@ -375,7 +375,7 @@  grid_inner_loop_gridifiable_p (gomp_for *gfor, grid_prop *grid)
 	      dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,
 			       GRID_MISSED_MSG_PREFIX "a reduction "
 			       "clause is present\n ");
-	      dump_printf_loc (MSG_NOTE, gimple_location (gfor),
+	      dump_printf_loc (MSG_NOTE, gfor,
 			       "Loop construct has a reduction schedule "
 			       "clause\n");
 	    }
@@ -404,7 +404,7 @@  grid_inner_loop_gridifiable_p (gomp_for *gfor, grid_prop *grid)
 			     GRID_MISSED_MSG_PREFIX "the inner loop contains "
 			     "statement %s which cannot be transformed\n",
 			     gimple_code_name[(int) gimple_code (bad)]);
-	  dump_printf_loc (MSG_NOTE, gimple_location (bad),
+	  dump_printf_loc (MSG_NOTE, bad,
 			   "This statement cannot be analyzed for "
 			   "gridification\n");
 	}
@@ -422,7 +422,7 @@  grid_inner_loop_gridifiable_p (gomp_for *gfor, grid_prop *grid)
 static bool
 grid_dist_follows_simple_pattern (gomp_for *dist, grid_prop *grid)
 {
-  location_t tloc = grid->target_loc;
+  dump_user_location_t tloc = grid->target_loc;
   gimple *stmt = grid_find_single_omp_among_assignments (gimple_omp_body (dist),
 							 grid, "distribute");
   gomp_parallel *par;
@@ -468,7 +468,7 @@  grid_gfor_follows_tiling_pattern (gomp_for *gfor, grid_prop *grid)
 	  dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,
 			   GRID_MISSED_MSG_PREFIX "an inner loop is not "
 			   "a simple for loop\n");
-	  dump_printf_loc (MSG_NOTE, gimple_location (gfor),
+	  dump_printf_loc (MSG_NOTE, gfor,
 			   "This statement is not a simple for loop\n");
 	}
       return false;
@@ -484,7 +484,7 @@  grid_gfor_follows_tiling_pattern (gomp_for *gfor, grid_prop *grid)
 	  dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,
 			   GRID_MISSED_MSG_PREFIX "an inner loop does not "
 			   "have use the same collapse clause\n");
-	  dump_printf_loc (MSG_NOTE, gimple_location (gfor),
+	  dump_printf_loc (MSG_NOTE, gfor,
 			   "Loop construct uses a different collapse clause\n");
 	}
       return false;
@@ -524,7 +524,7 @@  grid_gfor_follows_tiling_pattern (gomp_for *gfor, grid_prop *grid)
 	      dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,
 			       GRID_MISSED_MSG_PREFIX "the distribute and "
 			       "an internal loop do not agree on tile size\n");
-	      dump_printf_loc (MSG_NOTE, gimple_location (gfor),
+	      dump_printf_loc (MSG_NOTE, gfor,
 			       "Loop construct does not seem to loop over "
 			       "a tile size\n");
 	    }
@@ -636,7 +636,7 @@  grid_dist_follows_tiling_pattern (gimple_seq seq, grid_prop *grid,
 		  dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,
 				   GRID_MISSED_MSG_PREFIX "the distribute "
 				   "construct contains a try..catch region\n");
-		  dump_printf_loc (MSG_NOTE, gimple_location (try_stmt),
+		  dump_printf_loc (MSG_NOTE, try_stmt,
 				   "This statement cannot be analyzed for "
 				   "tiled gridification\n");
 		}
@@ -661,7 +661,7 @@  grid_dist_follows_tiling_pattern (gimple_seq seq, grid_prop *grid,
 	      dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,
 			       GRID_MISSED_MSG_PREFIX "the distribute "
 			       "construct contains a call\n");
-	      dump_printf_loc (MSG_NOTE, gimple_location (stmt),
+	      dump_printf_loc (MSG_NOTE, stmt,
 			       "This statement cannot be analyzed for "
 			       "tiled gridification\n");
 	    }
@@ -677,7 +677,7 @@  grid_dist_follows_tiling_pattern (gimple_seq seq, grid_prop *grid,
 				   GRID_MISSED_MSG_PREFIX "a parallel "
 				   "construct contains another parallel "
 				   "construct\n");
-		  dump_printf_loc (MSG_NOTE, gimple_location (stmt),
+		  dump_printf_loc (MSG_NOTE, stmt,
 				   "This parallel construct is nested in "
 				   "another one\n");
 		}
@@ -698,7 +698,7 @@  grid_dist_follows_tiling_pattern (gimple_seq seq, grid_prop *grid,
 				   GRID_MISSED_MSG_PREFIX "a loop "
 				   "construct is not nested within a parallel "
 				   "construct\n");
-		  dump_printf_loc (MSG_NOTE, gimple_location (stmt),
+		  dump_printf_loc (MSG_NOTE, stmt,
 				   "This loop construct is not nested in "
 				   "a parallel construct\n");
 		}
@@ -714,7 +714,7 @@  grid_dist_follows_tiling_pattern (gimple_seq seq, grid_prop *grid,
 	      dump_printf_loc (MSG_MISSED_OPTIMIZATION, grid->target_loc,
 			       GRID_MISSED_MSG_PREFIX "the distribute "
 			       "construct contains a complex statement\n");
-	      dump_printf_loc (MSG_NOTE, gimple_location (stmt),
+	      dump_printf_loc (MSG_NOTE, stmt,
 			       "This statement cannot be analyzed for "
 			       "tiled gridification\n");
 	    }
@@ -734,7 +734,7 @@  grid_target_follows_gridifiable_pattern (gomp_target *target, grid_prop *grid)
   if (gimple_omp_target_kind (target) != GF_OMP_TARGET_KIND_REGION)
     return false;
 
-  location_t tloc = gimple_location (target);
+  dump_user_location_t tloc = target;
   grid->target_loc = tloc;
   gimple *stmt
     = grid_find_single_omp_among_assignments (gimple_omp_body (target),
@@ -1257,14 +1257,13 @@  grid_attempt_target_gridification (gomp_target *target,
 				   gbind *tgt_bind)
 {
   /* removed group_size */
-  grid_prop grid;
-  memset (&grid, 0, sizeof (grid));
+  grid_prop grid = {};
   if (!target || !grid_target_follows_gridifiable_pattern (target, &grid))
     return;
 
   location_t loc = gimple_location (target);
   if (dump_enabled_p ())
-    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
+    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, target,
 		     "Target construct will be turned into a gridified HSA "
 		     "kernel\n");
 
diff --git a/gcc/profile.c b/gcc/profile.c
index 8ba6dc7..0cd0270 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -447,9 +447,14 @@  read_profile_edge_counts (gcov_type *exec_counts)
 		      {
 			static bool informed = 0;
 			if (dump_enabled_p () && !informed)
-		          dump_printf_loc (MSG_NOTE, input_location,
-                                           "corrupted profile info: edge count"
-                                           " exceeds maximal count\n");
+			  {
+			    dump_location_t loc
+			      = dump_location_t::from_location_t
+			        (input_location);
+			    dump_printf_loc (MSG_NOTE, loc,
+					     "corrupted profile info: edge count"
+					     " exceeds maximal count\n");
+			  }
 			informed = 1;
 		      }
 		    else
@@ -672,7 +677,8 @@  compute_branch_probabilities (unsigned cfg_checksum, unsigned lineno_checksum)
          if (dump_enabled_p () && informed == 0)
            {
              informed = 1;
-             dump_printf_loc (MSG_NOTE, input_location,
+             dump_printf_loc (MSG_NOTE,
+			      dump_location_t::from_location_t (input_location),
                               "correcting inconsistent profile data\n");
            }
          correct_negative_edge_counts ();
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index fe221ff..a9aacc02 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -59,6 +59,7 @@  selftest::run_tests ()
   /* Low-level data structures.  */
   bitmap_c_tests ();
   sbitmap_c_tests ();
+  dumpfile_c_tests ();
   et_forest_c_tests ();
   hash_map_tests_c_tests ();
   hash_set_tests_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index fc47b2c..a5507cc 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -188,6 +188,7 @@  extern void attribute_c_tests ();
 extern void bitmap_c_tests ();
 extern void diagnostic_c_tests ();
 extern void diagnostic_show_locus_c_tests ();
+extern void dumpfile_c_tests ();
 extern void edit_context_c_tests ();
 extern void et_forest_c_tests ();
 extern void fibonacci_heap_c_tests ();
diff --git a/gcc/tree-loop-distribution.c b/gcc/tree-loop-distribution.c
index c6e0a60..1206614 100644
--- a/gcc/tree-loop-distribution.c
+++ b/gcc/tree-loop-distribution.c
@@ -3117,7 +3117,7 @@  pass_loop_distribution::execute (function *fun)
 	    break;
 
 	  const char *str = loop->inner ? " nest" : "";
-	  location_t loc = find_loop_location (loop);
+	  dump_user_location_t loc = find_loop_location (loop);
 	  if (!cd)
 	    {
 	      calculate_dominance_info (CDI_DOMINATORS);
diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c
index c49f032..e79a954 100644
--- a/gcc/tree-parloops.c
+++ b/gcc/tree-parloops.c
@@ -3286,7 +3286,6 @@  parallelize_loops (bool oacc_kernels_p)
   struct tree_niter_desc niter_desc;
   struct obstack parloop_obstack;
   HOST_WIDE_INT estimated;
-  source_location loop_loc;
 
   /* Do not parallelize loops in the functions created by parallelization.  */
   if (!oacc_kernels_p
@@ -3411,7 +3410,7 @@  parallelize_loops (bool oacc_kernels_p)
       changed = true;
       skip_loop = loop->inner;
 
-      loop_loc = find_loop_location (loop);
+      dump_user_location_t loop_loc = find_loop_location (loop);
       if (loop->inner)
 	dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loop_loc,
 			 "parallelizing outer loop %d\n", loop->num);
diff --git a/gcc/tree-ssa-loop-ivcanon.c b/gcc/tree-ssa-loop-ivcanon.c
index 24bf60e..5f741c3 100644
--- a/gcc/tree-ssa-loop-ivcanon.c
+++ b/gcc/tree-ssa-loop-ivcanon.c
@@ -691,7 +691,7 @@  try_unroll_loop_completely (struct loop *loop,
 			    edge exit, tree niter, bool may_be_zero,
 			    enum unroll_level ul,
 			    HOST_WIDE_INT maxiter,
-			    location_t locus, bool allow_peel)
+			    dump_user_location_t locus, bool allow_peel)
 {
   unsigned HOST_WIDE_INT n_unroll = 0;
   bool n_unroll_found = false;
@@ -1162,7 +1162,7 @@  canonicalize_loop_induction_variables (struct loop *loop,
   tree niter;
   HOST_WIDE_INT maxiter;
   bool modified = false;
-  location_t locus = UNKNOWN_LOCATION;
+  dump_user_location_t locus;
   struct tree_niter_desc niter_desc;
   bool may_be_zero = false;
 
@@ -1177,7 +1177,7 @@  canonicalize_loop_induction_variables (struct loop *loop,
 	= niter_desc.may_be_zero && !integer_zerop (niter_desc.may_be_zero);
     }
   if (TREE_CODE (niter) == INTEGER_CST)
-    locus = gimple_location (last_stmt (exit->src));
+    locus = last_stmt (exit->src);
   else
     {
       /* For non-constant niter fold may_be_zero into niter again.  */
@@ -1204,7 +1204,7 @@  canonicalize_loop_induction_variables (struct loop *loop,
 	niter = find_loop_niter_by_eval (loop, &exit);
 
       if (exit)
-        locus = gimple_location (last_stmt (exit->src));
+        locus = last_stmt (exit->src);
 
       if (TREE_CODE (niter) != INTEGER_CST)
 	exit = NULL;
diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 519649a..6b445bd 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -7535,7 +7535,7 @@  tree_ssa_iv_optimize_loop (struct ivopts_data *data, struct loop *loop)
 
   gcc_assert (!data->niters);
   data->current_loop = loop;
-  data->loop_loc = find_loop_location (loop);
+  data->loop_loc = find_loop_location (loop).get_location_t ();
   data->speed = optimize_loop_for_speed_p (loop);
 
   if (dump_file && (dump_flags & TDF_DETAILS))
diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index f5ffc0f..03588a0 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -2627,7 +2627,7 @@  number_of_iterations_exit (struct loop *loop, edge exit,
     return true;
 
   if (warn)
-    dump_printf_loc (MSG_MISSED_OPTIMIZATION, gimple_location_safe (stmt),
+    dump_printf_loc (MSG_MISSED_OPTIMIZATION, stmt,
 		     "missed loop optimization: niters analysis ends up "
 		     "with assumptions.\n");
 
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 3d025c2..e5eddf9 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -5866,8 +5866,7 @@  eliminate_dom_walker::before_dom_children (basic_block b)
 		    fn = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
 		  if (dump_enabled_p ())
 		    {
-		      location_t loc = gimple_location (stmt);
-		      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc,
+		      dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt,
 				       "converting indirect call to "
 				       "function %s\n",
 				       lang_hooks.decl_printable_name (fn, 2));
diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
index f52ca0d..ea648f2 100644
--- a/gcc/tree-vect-loop-manip.c
+++ b/gcc/tree-vect-loop-manip.c
@@ -1301,7 +1301,7 @@  create_lcssa_for_virtual_phi (struct loop *loop)
    location is calculated.
    Return the loop location if succeed and NULL if not.  */
 
-source_location
+dump_user_location_t
 find_loop_location (struct loop *loop)
 {
   gimple *stmt = NULL;
@@ -1309,19 +1309,19 @@  find_loop_location (struct loop *loop)
   gimple_stmt_iterator si;
 
   if (!loop)
-    return UNKNOWN_LOCATION;
+    return dump_user_location_t ();
 
   stmt = get_loop_exit_condition (loop);
 
   if (stmt
       && LOCATION_LOCUS (gimple_location (stmt)) > BUILTINS_LOCATION)
-    return gimple_location (stmt);
+    return stmt;
 
   /* If we got here the loop is probably not "well formed",
      try to estimate the loop location */
 
   if (!loop->header)
-    return UNKNOWN_LOCATION;
+    return dump_user_location_t ();
 
   bb = loop->header;
 
@@ -1329,10 +1329,10 @@  find_loop_location (struct loop *loop)
     {
       stmt = gsi_stmt (si);
       if (LOCATION_LOCUS (gimple_location (stmt)) > BUILTINS_LOCATION)
-        return gimple_location (stmt);
+        return stmt;
     }
 
-  return UNKNOWN_LOCATION;
+  return dump_user_location_t ();
 }
 
 /* Return true if PHI defines an IV of the loop to be vectorized.  */
@@ -2498,7 +2498,7 @@  vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1,
 	}
     }
 
-  source_location loop_loc = find_loop_location (loop);
+  dump_user_location_t loop_loc = find_loop_location (loop);
   struct loop *scalar_loop = LOOP_VINFO_SCALAR_LOOP (loop_vinfo);
   if (prolog_peeling)
     {
@@ -3072,7 +3072,7 @@  vect_loop_versioning (loop_vec_info loop_vinfo,
       loop_constraint_set (loop, LOOP_C_INFINITE);
     }
 
-  if (LOCATION_LOCUS (vect_location) != UNKNOWN_LOCATION
+  if (LOCATION_LOCUS (vect_location.get_location_t ()) != UNKNOWN_LOCATION
       && dump_enabled_p ())
     {
       if (version_alias)
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 2b3ced2..b9ccae7 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -2731,8 +2731,8 @@  needs_fold_left_reduction_p (tree type, tree_code code,
    reduction operation CODE has a handled computation expression.  */
 
 bool
-check_reduction_path (location_t loc, loop_p loop, gphi *phi, tree loop_arg,
-		      enum tree_code code)
+check_reduction_path (dump_user_location_t loc, loop_p loop, gphi *phi,
+		      tree loop_arg, enum tree_code code)
 {
   auto_vec<std::pair<ssa_op_iter, use_operand_p> > path;
   auto_bitmap visited;
@@ -3750,8 +3750,8 @@  vect_estimate_min_profitable_iters (loop_vec_info loop_vinfo,
   else
     {
       if (LOOP_VINFO_LOOP (loop_vinfo)->force_vectorize)
-	warning_at (vect_location, OPT_Wopenmp_simd, "vectorization "
-		    "did not happen for a simd loop");
+	warning_at (vect_location.get_location_t (), OPT_Wopenmp_simd,
+		    "vectorization did not happen for a simd loop");
 
       if (dump_enabled_p ())
         dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 6739ade..31b58db 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -1510,7 +1510,8 @@  fail:
 /* Dump a slp tree NODE using flags specified in DUMP_KIND.  */
 
 static void
-vect_print_slp_tree (dump_flags_t dump_kind, location_t loc, slp_tree node)
+vect_print_slp_tree (dump_flags_t dump_kind, dump_location_t loc,
+		     slp_tree node)
 {
   int i;
   gimple *stmt;
@@ -3003,7 +3004,7 @@  vect_slp_bb (basic_block bb)
 	  insns++;
 
 	  if (gimple_location (stmt) != UNKNOWN_LOCATION)
-	    vect_location = gimple_location (stmt);
+	    vect_location = stmt;
 
 	  if (!vect_find_stmt_data_reference (NULL, stmt, &datarefs))
 	    break;
diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index 504a000..971221c 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -81,8 +81,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "gimple-pretty-print.h"
 
 
-/* Loop or bb location.  */
-source_location vect_location;
+/* Loop or bb location, with hotness information.  */
+dump_user_location_t vect_location;
 
 /* Vector mapping GIMPLE stmt to stmt_vec_info. */
 vec<stmt_vec_info> *stmt_vec_info_vec;
@@ -696,11 +696,11 @@  try_vectorize_loop_1 (hash_table<simduid_to_vf> *&simduid_to_vf_htab,
   unsigned ret = 0;
   vec_info_shared shared;
   vect_location = find_loop_location (loop);
-  if (LOCATION_LOCUS (vect_location) != UNKNOWN_LOCATION
+  if (LOCATION_LOCUS (vect_location.get_location_t ()) != UNKNOWN_LOCATION
       && dump_enabled_p ())
     dump_printf (MSG_NOTE, "\nAnalyzing loop at %s:%d\n",
-		 LOCATION_FILE (vect_location),
-		 LOCATION_LINE (vect_location));
+		 LOCATION_FILE (vect_location.get_location_t ()),
+		 LOCATION_LINE (vect_location.get_location_t ()));
 
   loop_vec_info loop_vinfo = vect_analyze_loop (loop, orig_loop_vinfo, &shared);
   loop->aux = loop_vinfo;
@@ -917,7 +917,7 @@  vectorize_loops (void)
       ret |= try_vectorize_loop (simduid_to_vf_htab, &num_vectorized_loops,
 				 loop);
 
-  vect_location = UNKNOWN_LOCATION;
+  vect_location = dump_user_location_t ();
 
   statistics_counter_event (cfun, "Vectorized loops", num_vectorized_loops);
   if (dump_enabled_p ()
@@ -1249,7 +1249,7 @@  increase_alignment (void)
 {
   varpool_node *vnode;
 
-  vect_location = UNKNOWN_LOCATION;
+  vect_location = dump_user_location_t ();
   type_align_map = new hash_map<tree, unsigned>;
 
   /* Increase the alignment of all global arrays for vectorization.  */
diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 55f8e6e..94a0f38 100644
--- a/gcc/tree-vectorizer.h
+++ b/gcc/tree-vectorizer.h
@@ -1437,8 +1437,8 @@  vect_get_scalar_dr_size (struct data_reference *dr)
   return tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (DR_REF (dr))));
 }
 
-/* Source location */
-extern source_location vect_location;
+/* Source location + hotness information. */
+extern dump_user_location_t vect_location;
 
 /* If dumping is enabled, emit a MSG_NOTE at vect_location about
    entering MSG within the vectorizer.  MSG should be a string literal. */
@@ -1466,7 +1466,7 @@  extern void vect_loop_versioning (loop_vec_info, unsigned int, bool,
 extern struct loop *vect_do_peeling (loop_vec_info, tree, tree,
 				     tree *, tree *, tree *, int, bool, bool);
 extern void vect_prepare_for_masked_peels (loop_vec_info);
-extern source_location find_loop_location (struct loop *);
+extern dump_user_location_t find_loop_location (struct loop *);
 extern bool vect_can_advance_ivs_p (loop_vec_info);
 
 /* In tree-vect-stmts.c.  */
@@ -1582,7 +1582,7 @@  extern tree vect_create_addr_base_for_vector_ref (gimple *, gimple_seq *,
 extern gimple *vect_force_simple_reduction (loop_vec_info, gimple *,
 					    bool *, bool);
 /* Used in gimple-loop-interchange.c.  */
-extern bool check_reduction_path (location_t, loop_p, gphi *, tree,
+extern bool check_reduction_path (dump_user_location_t, loop_p, gphi *, tree,
 				  enum tree_code);
 /* Drive for loop analysis stage.  */
 extern loop_vec_info vect_analyze_loop (struct loop *, loop_vec_info,
diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index d50a179..77d4849 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -585,10 +585,11 @@  check_counter (gimple *stmt, const char * name,
   gcov_type bb_count = bb_count_d.ipa ().to_gcov_type ();
   if (*all != bb_count || *count > *all)
     {
-      location_t locus;
-      locus = (stmt != NULL)
-              ? gimple_location (stmt)
-              : DECL_SOURCE_LOCATION (current_function_decl);
+      dump_user_location_t locus;
+      locus = ((stmt != NULL)
+	       ? dump_user_location_t (stmt)
+	       : dump_user_location_t::from_function_decl
+		   (current_function_decl));
       if (flag_profile_correction)
         {
           if (dump_enabled_p ())
@@ -603,7 +604,7 @@  check_counter (gimple *stmt, const char * name,
 	}
       else
 	{
-	  error_at (locus, "corrupted value profile: %s "
+	  error_at (locus.get_location_t (), "corrupted value profile: %s "
 		    "profile counter (%d out of %d) inconsistent with "
 		    "basic-block count (%d)",
 		    name,
@@ -1271,13 +1272,11 @@  find_func_by_profile_id (int profile_id)
 bool
 check_ic_target (gcall *call_stmt, struct cgraph_node *target)
 {
-   location_t locus;
    if (gimple_check_call_matching_types (call_stmt, target->decl, true))
      return true;
 
-   locus =  gimple_location (call_stmt);
    if (dump_enabled_p ())
-     dump_printf_loc (MSG_MISSED_OPTIMIZATION, locus,
+     dump_printf_loc (MSG_MISSED_OPTIMIZATION, call_stmt,
                       "Skipping target %s with mismatching types for icall\n",
                       target->name ());
    return false;