On Thu, Jan 10, 2019 at 2:55 AM David Malcolm <dmalc...@redhat.com> wrote:
>
> PR tree-optimization/88763 reports difficulties in getting useful
> information from the log of tree-ssa-loop-unswitch.c.
>
> As work towards improving this, this patch eliminates all uses of
> dump_file from that source file, in favor of the dump API (to
> support -fopt-info* and -fsave-optimization-record).
>
> Doing so required adding a way to dump profile_probability instances,
> so the patch adds a profile_probability::dump overload for this.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for next stage 1?
OK.
Thanks,
Richard.
> gcc/ChangeLog:
> PR tree-optimization/88763
> * cfgloop.h (find_loop_location): Move decl to here.
> * profile-count.c (profile_probability::dump): Reimplement in
> terms of...
> (profile_probability::print): ...this new function, moving the
> body here, and porting from FILE * to pretty_printer *.
> (profile_probability::dump): New function.
> * profile-count.h (class dump_metadata_t): New forward decl.
> (profile_probability::print): New decl.
> (profile_probability::dump): New overloaded decl.
> * tree-ssa-loop-unswitch.c (tree_unswitch_single_loop): Port from
> fprintf to the dump_* API.
> (tree_unswitch_outer_loop): Likewise.
> (find_loop_guard): Likewise.
> (hoist_guard): Likewise.
> * tree-vectorizer.h (find_loop_location): Move from here to
> cfgloop.h.
> ---
> gcc/cfgloop.h | 3 +
> gcc/profile-count.c | 34 ++++++++---
> gcc/profile-count.h | 7 +++
> gcc/tree-ssa-loop-unswitch.c | 135
> ++++++++++++++++++++++++++-----------------
> gcc/tree-vectorizer.h | 1 -
> 5 files changed, 120 insertions(+), 60 deletions(-)
>
> diff --git a/gcc/cfgloop.h b/gcc/cfgloop.h
> index 2e93af3..c175dcb 100644
> --- a/gcc/cfgloop.h
> +++ b/gcc/cfgloop.h
> @@ -870,4 +870,7 @@ gcov_type_to_wide_int (gcov_type val)
>
> return widest_int::from_array (a, 2);
> }
> +
> +extern dump_user_location_t find_loop_location (struct loop *);
> +
> #endif /* GCC_CFGLOOP_H */
> diff --git a/gcc/profile-count.c b/gcc/profile-count.c
> index 8c58f86..eaad18d 100644
> --- a/gcc/profile-count.c
> +++ b/gcc/profile-count.c
> @@ -148,27 +148,47 @@ profile_count::stream_out (struct lto_output_stream *ob)
> void
> profile_probability::dump (FILE *f) const
> {
> + pretty_printer pp;
> + print (&pp);
> + fprintf (f, "%s", pp_formatted_text (&pp));
> +}
> +
> +/* Print THIS to the PP. */
> +
> +void
> +profile_probability::print (pretty_printer *pp) const
> +{
> if (!initialized_p ())
> - fprintf (f, "uninitialized");
> + pp_string (pp, "uninitialized");
> else
> {
> /* Make difference between 0.00 as a roundoff error and actual 0.
> Similarly for 1. */
> if (m_val == 0)
> - fprintf (f, "never");
> + pp_string (pp, "never");
> else if (m_val == max_probability)
> - fprintf (f, "always");
> + pp_string (pp, "always");
> else
> - fprintf (f, "%3.1f%%", (double)m_val * 100 / max_probability);
> + pp_scalar (pp, "%3.1f%%", (double)m_val * 100 / max_probability);
> if (m_quality == profile_adjusted)
> - fprintf (f, " (adjusted)");
> + pp_string (pp, " (adjusted)");
> else if (m_quality == profile_afdo)
> - fprintf (f, " (auto FDO)");
> + pp_string (pp, " (auto FDO)");
> else if (m_quality == profile_guessed)
> - fprintf (f, " (guessed)");
> + pp_string (pp, " (guessed)");
> }
> }
>
> +/* Dump THIS to the active dump destinations. */
> +
> +void
> +profile_probability::dump (const dump_metadata_t &metadata) const
> +{
> + pretty_printer pp;
> + print (&pp);
> + dump_printf (metadata, "%s", pp_formatted_text (&pp));
> +}
> +
> /* Dump THIS to stderr. */
>
> void
> diff --git a/gcc/profile-count.h b/gcc/profile-count.h
> index 96b7400..a761836 100644
> --- a/gcc/profile-count.h
> +++ b/gcc/profile-count.h
> @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see
>
> struct function;
> class profile_count;
> +class dump_metadata_t;
>
> /* Quality of the profile count. Because gengtype does not support enums
> inside of classes, this is in global namespace. */
> @@ -561,6 +562,12 @@ public:
> /* Output THIS to F. */
> void dump (FILE *f) const;
>
> + /* Output THIS to the active dump destinations. */
> + void dump (const dump_metadata_t &) const;
> +
> + /* Write THIS to PP. */
> + void print (pretty_printer *pp) const;
> +
> /* Print THIS to stderr. */
> void debug () const;
>
> diff --git a/gcc/tree-ssa-loop-unswitch.c b/gcc/tree-ssa-loop-unswitch.c
> index 30a2a9d..3fdbc57 100644
> --- a/gcc/tree-ssa-loop-unswitch.c
> +++ b/gcc/tree-ssa-loop-unswitch.c
> @@ -281,8 +281,10 @@ tree_unswitch_single_loop (struct loop *loop, int num)
> /* Do not unswitch in cold regions. */
> if (optimize_loop_for_size_p (loop))
> {
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, ";; Not unswitching cold loops\n");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION | TDF_DETAILS,
> + find_loop_location (loop),
> + ";; Not unswitching cold loops\n");
> return false;
> }
>
> @@ -290,8 +292,10 @@ tree_unswitch_single_loop (struct loop *loop, int num)
> if (tree_num_loop_insns (loop, &eni_size_weights)
> > (unsigned) PARAM_VALUE (PARAM_MAX_UNSWITCH_INSNS))
> {
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, ";; Not unswitching, loop too big\n");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION | TDF_DETAILS,
> + find_loop_location (loop),
> + ";; Not unswitching, loop too big\n");
> return false;
> }
>
> @@ -302,9 +306,11 @@ tree_unswitch_single_loop (struct loop *loop, int num)
> iterations = likely_max_loop_iterations_int (loop);
> if (iterations >= 0 && iterations <= 1)
> {
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, ";; Not unswitching, loop is not expected"
> - " to iterate\n");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION | TDF_DETAILS,
> + find_loop_location (loop),
> + ";; Not unswitching, loop is not expected"
> + " to iterate\n");
> return false;
> }
> }
> @@ -322,10 +328,11 @@ tree_unswitch_single_loop (struct loop *loop, int num)
>
> if (i == loop->num_nodes)
> {
> - if (dump_file
> - && num > PARAM_VALUE (PARAM_MAX_UNSWITCH_LEVEL)
> - && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, ";; Not unswitching anymore, hit max
> level\n");
> + if (dump_enabled_p ())
> + if (num > PARAM_VALUE (PARAM_MAX_UNSWITCH_LEVEL))
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION | TDF_DETAILS,
> + find_loop_location (loop),
> + ";; Not unswitching anymore, hit max level\n");
>
> if (found == loop->num_nodes)
> {
> @@ -447,8 +454,10 @@ tree_unswitch_single_loop (struct loop *loop, int num)
> }
> }
>
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, ";; Unswitching loop\n");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_OPTIMIZED_LOCATIONS | TDF_DETAILS,
> + find_loop_location (loop),
> + ";; Unswitching loop\n");
>
> initialize_original_copy_tables ();
> /* Unswitch the loop on this condition. */
> @@ -522,9 +531,11 @@ tree_unswitch_outer_loop (struct loop *loop)
> iterations = likely_max_loop_iterations_int (loop);
> if (iterations >= 0 && iterations <= 1)
> {
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, ";; Not unswitching, loop is not expected"
> - " to iterate\n");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION | TDF_DETAILS,
> + find_loop_location (loop),
> + ";; Not unswitching, loop is not expected"
> + " to iterate\n");
> return false;
> }
>
> @@ -629,22 +640,27 @@ find_loop_guard (struct loop *loop)
> if (!dominated_by_p (CDI_DOMINATORS, loop->inner->header,
> guard_edge == fe ? te->dest : fe->dest))
> {
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, "Guard edge %d --> %d is not around the loop!\n",
> - guard_edge->src->index, guard_edge->dest->index);
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION | TDF_DETAILS,
> + find_loop_location (loop),
> + "Guard edge %d --> %d is not around the loop!\n",
> + guard_edge->src->index, guard_edge->dest->index);
> return NULL;
> }
> if (guard_edge->dest == loop->latch)
> {
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, "Guard edge destination is loop latch.\n");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION | TDF_DETAILS,
> + find_loop_location (loop),
> + "Guard edge destination is loop latch.\n");
> return NULL;
> }
>
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file,
> - "Considering guard %d -> %d in loop %d\n",
> - guard_edge->src->index, guard_edge->dest->index, loop->num);
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_NOTE | TDF_DETAILS, find_loop_location (loop),
> + "Considering guard %d -> %d in loop %d\n",
> + guard_edge->src->index, guard_edge->dest->index,
> + loop->num);
> /* Check if condition operands do not have definitions inside loop since
> any bb copying is not performed. */
> FOR_EACH_SSA_TREE_OPERAND (use, cond, iter, SSA_OP_USE)
> @@ -654,9 +670,10 @@ find_loop_guard (struct loop *loop)
> if (def_bb
> && flow_bb_inside_loop_p (loop, def_bb))
> {
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, " guard operands have definitions"
> - " inside loop\n");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION | TDF_DETAILS, def,
> + " guard operands have definitions"
> + " inside loop\n");
> return NULL;
> }
> }
> @@ -669,23 +686,28 @@ find_loop_guard (struct loop *loop)
> continue;
> if (bb->flags & BB_IRREDUCIBLE_LOOP)
> {
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, "Block %d is marked as irreducible in loop\n",
> - bb->index);
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION | TDF_DETAILS,
> + find_loop_location (loop),
> + "Block %d is marked as irreducible in loop\n",
> + bb->index);
> guard_edge = NULL;
> goto end;
> }
> if (!empty_bb_without_guard_p (loop, bb))
> {
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, " block %d has side effects\n", bb->index);
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION | TDF_DETAILS,
> + find_loop_location (loop),
> + " block %d has side effects\n", bb->index);
> guard_edge = NULL;
> goto end;
> }
> }
>
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, " suitable to hoist\n");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_NOTE | TDF_DETAILS, find_loop_location (loop),
> + " suitable to hoist\n");
> end:
> if (body)
> free (body);
> @@ -824,13 +846,15 @@ hoist_guard (struct loop *loop, edge guard)
> update_stmt (cond_stmt);
> /* Create new loop pre-header. */
> e = split_block (pre_header, last_stmt (pre_header));
> - if (dump_file && (dump_flags & TDF_DETAILS))
> + if (dump_enabled_p ())
> {
> - fprintf (dump_file, " Moving guard %i->%i (prob ",
> - guard->src->index, guard->dest->index);
> - guard->probability.dump (dump_file);
> - fprintf (dump_file, ") to bb %i, new preheader is %i\n",
> - e->src->index, e->dest->index);
> + dump_printf_loc (MSG_NOTE | TDF_DETAILS, cond_stmt,
> + " Moving guard %i->%i (prob ",
> + guard->src->index, guard->dest->index);
> + guard->probability.dump (MSG_NOTE | TDF_DETAILS);
> + dump_printf (MSG_NOTE | TDF_DETAILS,
> + ") to bb %i, new preheader is %i\n",
> + e->src->index, e->dest->index);
> }
>
> gcc_assert (loop_preheader_edge (loop)->src == e->dest);
> @@ -859,14 +883,17 @@ hoist_guard (struct loop *loop, edge guard)
>
> if (skip_count > e->count ())
> {
> - fprintf (dump_file, " Capping count; expect profile inconsistency\n");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_NOTE | TDF_DETAILS, cond_stmt,
> + " Capping count; expect profile inconsistency\n");
> skip_count = e->count ();
> }
> - if (dump_file && (dump_flags & TDF_DETAILS))
> + if (dump_enabled_p ())
> {
> - fprintf (dump_file, " Estimated probability of skipping loop is ");
> - new_edge->probability.dump (dump_file);
> - fprintf (dump_file, "\n");
> + dump_printf_loc (MSG_NOTE | TDF_DETAILS, cond_stmt,
> + " Estimated probability of skipping loop is ");
> + new_edge->probability.dump (MSG_NOTE | TDF_DETAILS);
> + dump_printf (MSG_NOTE | TDF_DETAILS, "\n");
> }
>
> /* Update profile after the transform:
> @@ -885,19 +912,22 @@ hoist_guard (struct loop *loop, edge guard)
> where profile does not change. */
> basic_block *body = get_loop_body (loop);
>
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, " Scaling nonguarded BBs in loop:");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_NOTE | TDF_DETAILS, find_loop_location (loop),
> + " Scaling nonguarded BBs in loop:");
> for (unsigned int i = 0; i < loop->num_nodes; i++)
> {
> basic_block bb = body[i];
> if (!dominated_by_p (CDI_DOMINATORS, bb, not_guard->dest))
> {
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, " %i", bb->index);
> + if (dump_enabled_p ())
> + dump_printf (MSG_NOTE | TDF_DETAILS, " %i", bb->index);
> if (e->probability.initialized_p ())
> scale_bbs_frequencies (&bb, 1, e->probability);
> }
> }
> + if (dump_enabled_p ())
> + dump_printf (MSG_NOTE | TDF_DETAILS, "\n");
>
> if (fix_dom_of_exit)
> set_immediate_dominator (CDI_DOMINATORS, exit->dest, pre_header);
> @@ -924,8 +954,9 @@ hoist_guard (struct loop *loop, edge guard)
> }
> }
>
> - if (dump_file && (dump_flags & TDF_DETAILS))
> - fprintf (dump_file, "\n guard hoisted.\n");
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_OPTIMIZED_LOCATIONS | TDF_DETAILS,
> + cond_stmt, " guard hoisted.\n");
>
> free (body);
> }
> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index d26b0f8..14489ba 100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -1436,7 +1436,6 @@ 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 dump_user_location_t find_loop_location (struct loop *);
> extern bool vect_can_advance_ivs_p (loop_vec_info);
>
> /* In tree-vect-stmts.c. */
> --
> 1.8.5.3
>
