On Tue, Oct 18, 2011 at 3:48 PM, Rong Xu <x...@google.com> wrote:
> Suppress verbose notes/warnings printed in FDO-use compilation.
> (1) Add option -fprofile-use-verbose.
Gcc currently does not emit informational messages on high level
transformations such as inlining, value profiling transformations (ic
promotion info messages are emitted in lipo mode in google/main), loop
peeling and unrolling, and other loop opts. I can see those are very
useful for triaging performance regressions etc, but turning those on
would be too verbose for non-power users and can be annoying. They
(when introduced in the future) should be guarded. I can see they
should be guarded using the same option and possibly with a verbose
level. Something like:
-fopt-info=<1,2,3>
For now, a nonparameterized option should be good enough.
(similarly, -fopt-report can be used to dump optimization report which
is more structured -- e.g, grouped via optimizations, not functions).
There are some unrelated changes (e.g, histogram free etc) which
should be committed separately.
David
> When this option is on, FDO-use
> compilation prints out all the notes as that of today. When this
> option is off (the default), all notes are suppressed.
> (2) Make several unconditional warnings under OPT_Wcoverage_mismatch.
> So that they can be turned off via -Wno-coverage-mismatch.
> (3) Make several unconditional warnings for LIPO under flag_ripa_verbose.
> (can be turned on via -fripa-verbose).
>
> This patch is for google-main only.
>
> Tested with bootstrap and regression tests.
>
> 2011-10-18 Rong Xu <x...@google.com>
>
> * gcc/common.opt (fprofile-use-verbose): New flag.
> * gcc/value-prof.c (check_ic_counter): guard notes by
> flag_profile_use_verbose.
> (find_func_by_funcdef_no): Ditto.
> (check_ic_target): Ditto.
> (check_counter): Ditto.
> (check_ic_counter): Ditto.
> * gcc/mcf.c (find_minimum_cost_flow): Ditto.
> * gcc/profile.c (read_profile_edge_counts):
> (compute_branch_probabilities):
> * gcc/coverage.c (read_counts_file): guard LIPO
> warnings by flag_ripa_verbose.
> (get_coverage_counts): guard notes
> by flag_profile_use_verbose; make warning
> under OPT_Wcoverage_mismatch.
> * gcc/tree-profile.c: (gimple_gen_reusedist): Ditto.
> (maybe_issue_profile_use_note): Ditto.
> (optimize_reusedist): Ditto.
> * gcc/testsuite/gcc.dg/pr32773.c: add -fprofile-use-verbose.
> * gcc/testsuite/gcc.dg/pr40209.c: Ditto.
> * gcc/testsuite/gcc.dg/pr26570.c: Ditto.
> * gcc/testsuite/g++.dg/tree-ssa/dom-invalid.C: Ditto.
>
> Index: gcc/value-prof.c
> ===================================================================
> --- gcc/value-prof.c (revision 180106)
> +++ gcc/value-prof.c (working copy)
> @@ -472,9 +472,10 @@
> : DECL_SOURCE_LOCATION (current_function_decl);
> if (flag_profile_correction)
> {
> - inform (locus, "correcting inconsistent value profile: "
> - "%s profiler overall count (%d) does not match BB count "
> - "(%d)", name, (int)*all, (int)bb_count);
> + if (flag_profile_use_verbose)
> + inform (locus, "correcting inconsistent value profile: %s "
> + "profiler overall count (%d) does not match BB count "
> + "(%d)", name, (int)*all, (int)bb_count);
> *all = bb_count;
> if (*count > *all)
> *count = *all;
> @@ -510,33 +511,42 @@
> location_t locus;
> if (*count1 > all && flag_profile_correction)
> {
> - locus = (stmt != NULL)
> - ? gimple_location (stmt)
> - : DECL_SOURCE_LOCATION (current_function_decl);
> - inform (locus, "Correcting inconsistent value profile: "
> - "ic (topn) profiler top target count (%ld) exceeds "
> - "BB count (%ld)", (long)*count1, (long)all);
> + if (flag_profile_use_verbose)
> + {
> + locus = (stmt != NULL)
> + ? gimple_location (stmt)
> + : DECL_SOURCE_LOCATION (current_function_decl);
> + inform (locus, "Correcting inconsistent value profile: "
> + "ic (topn) profiler top target count (%ld) exceeds "
> + "BB count (%ld)", (long)*count1, (long)all);
> + }
> *count1 = all;
> }
> if (*count2 > all && flag_profile_correction)
> {
> - locus = (stmt != NULL)
> - ? gimple_location (stmt)
> - : DECL_SOURCE_LOCATION (current_function_decl);
> - inform (locus, "Correcting inconsistent value profile: "
> - "ic (topn) profiler second target count (%ld) exceeds "
> - "BB count (%ld)", (long)*count2, (long)all);
> + if (flag_profile_use_verbose)
> + {
> + locus = (stmt != NULL)
> + ? gimple_location (stmt)
> + : DECL_SOURCE_LOCATION (current_function_decl);
> + inform (locus, "Correcting inconsistent value profile: "
> + "ic (topn) profiler second target count (%ld) exceeds "
> + "BB count (%ld)", (long)*count2, (long)all);
> + }
> *count2 = all;
> }
>
> if (*count2 > *count1)
> {
> - locus = (stmt != NULL)
> - ? gimple_location (stmt)
> - : DECL_SOURCE_LOCATION (current_function_decl);
> - inform (locus, "Corrupted topn ic value profile: "
> - "first target count (%ld) is less than the second "
> - "target count (%ld)", (long)*count1, (long)*count2);
> + if (flag_profile_use_verbose)
> + {
> + locus = (stmt != NULL)
> + ? gimple_location (stmt)
> + : DECL_SOURCE_LOCATION (current_function_decl);
> + inform (locus, "Corrupted topn ic value profile: "
> + "first target count (%ld) is less than the second "
> + "target count (%ld)", (long)*count1, (long)*count2);
> + }
> return true;
> }
>
> @@ -548,12 +558,16 @@
> *count2 = all - *count1;
> else
> {
> - locus = (stmt != NULL)
> - ? gimple_location (stmt)
> - : DECL_SOURCE_LOCATION (current_function_decl);
> - inform (locus, "Corrupted topn ic value profile: top two targets's"
> - " total count (%ld) exceeds bb count (%ld)",
> - (long)(*count1 + *count2), (long)all);
> + if (flag_profile_use_verbose)
> + {
> + locus = (stmt != NULL)
> + ? gimple_location (stmt)
> + : DECL_SOURCE_LOCATION (current_function_decl);
> + inform (locus,
> + "Corrupted topn ic value profile: top two targets's"
> + " total count (%ld) exceeds bb count (%ld)",
> + (long)(*count1 + *count2), (long)all);
> + }
> return true;
> }
> }
> @@ -1177,8 +1191,11 @@
> func_id) == NULL)
> {
> if (flag_profile_correction)
> - inform (DECL_SOURCE_LOCATION (current_function_decl),
> + {
> + if (flag_profile_use_verbose)
> + inform (DECL_SOURCE_LOCATION (current_function_decl),
> "Inconsistent profile: indirect call target (%d) does not
> exist", func_id);
> + }
> else
> error ("Inconsistent profile: indirect call target (%d) does not
> exist", func_id);
>
> @@ -1308,8 +1325,9 @@
> return true;
>
> locus = gimple_location (call_stmt);
> - inform (locus, "Skipping target %s with mismatching types for icall ",
> - cgraph_node_name (target));
> + if (flag_profile_use_verbose)
> + inform (locus, "Skipping target %s with mismatching types for icall ",
> + cgraph_node_name (target));
> return false;
> }
>
> Index: gcc/mcf.c
> ===================================================================
> --- gcc/mcf.c (revision 180106)
> +++ gcc/mcf.c (working copy)
> @@ -1442,7 +1442,8 @@
> if (iteration > MAX_ITER (fixup_graph->num_vertices,
> fixup_graph->num_edges))
> {
> - inform (DECL_SOURCE_LOCATION (current_function_decl),
> + if (flag_profile_use_verbose)
> + inform (DECL_SOURCE_LOCATION (current_function_decl),
> "Exiting profile correction early to avoid excessive "
> "compile time");
> break;
> Index: gcc/testsuite/gcc.dg/pr32773.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/pr32773.c (revision 180106)
> +++ gcc/testsuite/gcc.dg/pr32773.c (working copy)
> @@ -1,6 +1,6 @@
> /* { dg-do compile } */
> -/* { dg-options "-O -fprofile-use" } */
> -/* { dg-options "-O -m4 -fprofile-use" { target sh-*-* } } */
> +/* { dg-options "-O -fprofile-use -fprofile-use-verbose" } */
> +/* { dg-options "-O -m4 -fprofile-use -fprofile-use-verbose" { target sh-*-*
> } } */
>
> void foo (int *p)
> {
> Index: gcc/testsuite/gcc.dg/pr40209.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/pr40209.c (revision 180106)
> +++ gcc/testsuite/gcc.dg/pr40209.c (working copy)
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-O2 -fprofile-use" } */
> +/* { dg-options "-O2 -fprofile-use -fprofile-use-verbose" } */
>
> void process(const char *s);
>
> Index: gcc/testsuite/gcc.dg/pr26570.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/pr26570.c (revision 180106)
> +++ gcc/testsuite/gcc.dg/pr26570.c (working copy)
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-O2 -fprofile-generate -fprofile-use" } */
> +/* { dg-options "-O2 -fprofile-generate -fprofile-use -fprofile-use-verbose"
> } */
>
> unsigned test (unsigned a, unsigned b)
> {
> Index: gcc/testsuite/g++.dg/tree-ssa/dom-invalid.C
> ===================================================================
> --- gcc/testsuite/g++.dg/tree-ssa/dom-invalid.C (revision 180106)
> +++ gcc/testsuite/g++.dg/tree-ssa/dom-invalid.C (working copy)
> @@ -1,7 +1,7 @@
> // PR tree-optimization/39557
> // invalid post-dom info leads to infinite loop
> // { dg-do run }
> -// { dg-options "-Wall -fno-exceptions -O2 -fprofile-use -fno-rtti" }
> +// { dg-options "-Wall -fno-exceptions -O2 -fprofile-use
> -fprofile-use-verbose -fno-rtti" }
>
> struct C
> {
> Index: gcc/profile.c
> ===================================================================
> --- gcc/profile.c (revision 180106)
> +++ gcc/profile.c (working copy)
> @@ -414,7 +414,7 @@
> if (flag_profile_correction)
> {
> static bool informed = 0;
> - if (!informed)
> + if (flag_profile_use_verbose && !informed)
> inform (input_location,
> "corrupted profile info: edge count exceeds
> maximal count");
> informed = 1;
> @@ -635,7 +635,7 @@
> {
> /* Inconsistency detected. Make it flow-consistent. */
> static int informed = 0;
> - if (informed == 0)
> + if (flag_profile_use_verbose && informed == 0)
> {
> informed = 1;
> inform (input_location, "correcting inconsistent profile data");
> @@ -754,7 +754,8 @@
> }
> counts_to_freqs ();
> profile_status = PROFILE_READ;
> - compute_function_frequency ();
> + /* TODO: investigate performance regression with this.
> + compute_function_frequency (); */
>
> if (dump_file)
> {
> @@ -837,7 +838,8 @@
> }
>
> for (t = 0; t < GCOV_N_VALUE_COUNTERS; t++)
> - free (histogram_counts[t]);
> + if (histogram_counts[t])
> + free (histogram_counts[t]);
> }
>
> /* The entry basic block will be moved around so that it has index=1,
> @@ -861,7 +863,7 @@
> return;
> }
>
> - name_differs = !prev_file_name || filename_cmp (file_name, prev_file_name);
> + name_differs = !prev_file_name || strcmp (file_name, prev_file_name);
> line_differs = prev_line != line;
>
> if (name_differs || line_differs)
> @@ -1140,14 +1142,17 @@
> /* Line numbers. */
> if (coverage_begin_output (lineno_checksum, cfg_checksum))
> {
> + gcov_position_t offset;
> +
> /* Initialize the output. */
> output_location (NULL, 0, NULL, NULL);
>
> FOR_EACH_BB (bb)
> {
> gimple_stmt_iterator gsi;
> - gcov_position_t offset = 0;
>
> + offset = 0;
> +
> if (bb == ENTRY_BLOCK_PTR->next_bb)
> {
> expanded_location curr_location =
> @@ -1164,14 +1169,15 @@
> &offset, bb);
> }
>
> - /* Notice GOTO expressions eliminated while constructing the CFG.
> */
> + /* Notice GOTO expressions we eliminated while constructing the
> + CFG. */
> if (single_succ_p (bb)
> && single_succ_edge (bb)->goto_locus != UNKNOWN_LOCATION)
> {
> - expanded_location curr_location
> - = expand_location (single_succ_edge (bb)->goto_locus);
> - output_location (curr_location.file, curr_location.line,
> - &offset, bb);
> + location_t curr_location = single_succ_edge (bb)->goto_locus;
> + /* ??? The FILE/LINE API is inconsistent for these cases. */
> + output_location (LOCATION_FILE (curr_location),
> + LOCATION_LINE (curr_location), &offset, bb);
> }
>
> if (offset)
> Index: gcc/coverage.c
> ===================================================================
> --- gcc/coverage.c (revision 180106)
> +++ gcc/coverage.c (working copy)
> @@ -575,29 +575,47 @@
> int fd;
> char *aux_da_filename = get_da_file_name (mod_info->da_filename);
> gcc_assert (!mod_info->is_primary);
> - if (pointer_set_insert (modset, (void
> *)(size_t)mod_info->ident))
> - inform (input_location, "Not importing %s: already imported",
> - mod_info->source_filename);
> - else if ((module_infos[0]->lang & GCOV_MODULE_LANG_MASK) !=
> - (mod_info->lang & GCOV_MODULE_LANG_MASK))
> - inform (input_location, "Not importing %s: source language"
> - " different from primary module's source language",
> - mod_info->source_filename);
> - else if (module_infos_read == max_group)
> - inform (input_location, "Not importing %s: maximum group size"
> - " reached", mod_info->source_filename);
> - else if (incompatible_cl_args (module_infos[0], mod_info))
> - inform (input_location, "Not importing %s: command-line"
> - " arguments not compatible with primary module",
> - mod_info->source_filename);
> - else if ((fd = open (aux_da_filename, O_RDONLY)) < 0)
> - inform (input_location, "Not importing %s: couldn't open %s",
> - mod_info->source_filename, aux_da_filename);
> - else if ((mod_info->lang & GCOV_MODULE_ASM_STMTS)
> - && flag_ripa_disallow_asm_modules)
> - inform (input_location, "Not importing %s: contains assembler"
> - " statements", mod_info->source_filename);
> - else
> + if (pointer_set_insert (modset, (void
> *)(size_t)mod_info->ident))
> + {
> + if (flag_ripa_verbose)
> + inform (input_location, "Not importing %s: already
> imported",
> + mod_info->source_filename);
> + }
> + else if ((module_infos[0]->lang & GCOV_MODULE_LANG_MASK) !=
> + (mod_info->lang & GCOV_MODULE_LANG_MASK))
> + {
> + if (flag_ripa_verbose)
> + inform (input_location, "Not importing %s: source
> language"
> + " different from primary module's source
> language",
> + mod_info->source_filename);
> + }
> + else if (module_infos_read == max_group)
> + {
> + if (flag_ripa_verbose)
> + inform (input_location, "Not importing %s: maximum group"
> + " size reached", mod_info->source_filename);
> + }
> + else if (incompatible_cl_args (module_infos[0], mod_info))
> + {
> + if (flag_ripa_verbose)
> + inform (input_location, "Not importing %s: command-line"
> + " arguments not compatible with primary module",
> + mod_info->source_filename);
> + }
> + else if ((fd = open (aux_da_filename, O_RDONLY)) < 0)
> + {
> + if (flag_ripa_verbose)
> + inform (input_location, "Not importing %s: couldn't open
> %s",
> + mod_info->source_filename, aux_da_filename);
> + }
> + else if ((mod_info->lang & GCOV_MODULE_ASM_STMTS)
> + && flag_ripa_disallow_asm_modules)
> + {
> + if (flag_ripa_verbose)
> + inform (input_location, "Not importing %s: contains "
> + "assembler statements",
> mod_info->source_filename);
> + }
> + else
> {
> close (fd);
> module_infos_read++;
> @@ -676,7 +694,7 @@
> {
> static int warned = 0;
>
> - if (!warned++)
> + if (flag_profile_use_verbose && !warned++)
> inform (input_location, (flag_guess_branch_prob
> ? "file %s not found, execution counts estimated"
> : "file %s not found, execution counts assumed to be zero"),
> @@ -689,7 +707,8 @@
> if (!entry)
> {
> if (!flag_dyn_ipa)
> - warning (0, "no coverage for function %qE found",
> + warning (OPT_Wcoverage_mismatch,
> + "no coverage for function %qE found",
> DECL_ASSEMBLER_NAME (current_function_decl));
> return NULL;
> }
> @@ -705,7 +724,7 @@
> warning_at (input_location, OPT_Wcoverage_mismatch,
> "The control flow of function %qE does not match "
> "its profile data (counter %qs)", id, ctr_names[counter]);
> - if (warning_printed)
> + if (flag_profile_use_verbose && warning_printed)
> {
> inform (input_location, "Use -Wno-error=coverage-mismatch to tolerate
> "
> "the mismatch but performance may drop if the function is
> hot");
> @@ -727,7 +746,8 @@
> }
> else if (entry->lineno_checksum != lineno_checksum)
> {
> - warning (0, "Source location for function %qE have changed,"
> + warning (OPT_Wcoverage_mismatch,
> + "Source location for function %qE have changed,"
> " the profile data may be out of date",
> DECL_ASSEMBLER_NAME (current_function_decl));
> }
> Index: gcc/common.opt
> ===================================================================
> --- gcc/common.opt (revision 180106)
> +++ gcc/common.opt (working copy)
> @@ -1697,6 +1697,10 @@
> Common Joined RejectNegative
> Enable common options for performing profile feedback directed
> optimizations, and set -fprofile-dir=
>
> +fprofile-use-verbose
> +Common Report Var(flag_profile_use_verbose)
> +Enable verbose informational messages for FDO use compilation
> +
> fprofile-values
> Common Report Var(flag_profile_values)
> Insert code to profile values of expressions
> Index: gcc/tree-profile.c
> ===================================================================
> --- gcc/tree-profile.c (revision 180106)
> +++ gcc/tree-profile.c (working copy)
> @@ -1096,13 +1096,16 @@
> reusedist_make_instr_call (stmt, subst, counters),
> GSI_NEW_STMT);
>
> - locus = (stmt != NULL)
> - ? gimple_location (stmt)
> - : DECL_SOURCE_LOCATION (current_function_decl);
> - inform (locus,
> - "inserted reuse distance instrumentation for %qs, using "
> - "%d gcov counters", subst->original_name,
> - subst->num_ptr_args * RD_NUM_COUNTERS);
> + if (flag_profile_use_verbose)
> + {
> + locus = (stmt != NULL)
> + ? gimple_location (stmt)
> + : DECL_SOURCE_LOCATION (current_function_decl);
> + inform (locus,
> + "inserted reuse distance instrumentation for %qs,
> using "
> + "%d gcov counters", subst->original_name,
> + subst->num_ptr_args * RD_NUM_COUNTERS);
> + }
> }
> }
> }
> @@ -1214,7 +1217,7 @@
>
> reusedist_from_counters (counters, &rd);
>
> - if (rd.count)
> + if (flag_profile_use_verbose && rd.count)
> inform (locus, "reuse distance counters for arg %d: %lld %lld %lld %lld",
> arg, (long long int)rd.mean_dist, (long long int)rd.mean_size,
> (long long int)rd.count, (long long int)rd.dist_x_size);
> @@ -1283,9 +1286,10 @@
> subst_decl = reusedist_get_nt_decl (gimple_call_fndecl (stmt), subst,
> suffix);
> gimple_call_set_fndecl (stmt, subst_decl);
> - inform (locus, "replaced %qs with non-temporal %qs",
> - subst->original_name,
> - IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (subst_decl)));
> + if (flag_profile_use_verbose)
> + inform (locus, "replaced %qs with non-temporal %qs",
> + subst->original_name,
> + IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (subst_decl)));
> }
>
> /* Replace string operations with equivalent nontemporal, when profitable.
> */
> @@ -1323,11 +1327,13 @@
>
> if (counter_index != n_counters)
> {
> - warning (0, "coverage mismatch for reuse distance counters "
> + warning (OPT_Wcoverage_mismatch,
> + "coverage mismatch for reuse distance counters "
> "in function %qs", IDENTIFIER_POINTER
> (DECL_ASSEMBLER_NAME (current_function_decl)));
> - inform (input_location, "number of counters is %u instead of %u",
> - n_counters, counter_index);
> + if (flag_profile_use_verbose)
> + inform (input_location, "number of counters is %u instead of %u",
> + n_counters, counter_index);
> }
> }
>
>
> --
> This patch is available for review at http://codereview.appspot.com/5294043
>
