This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] decide edge's hotness when there is profile info
- From: Jan Hubicka <hubicka at ucw dot cz>
- To: Teresa Johnson <tejohnson at google dot com>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, Dehao Chen <dehao at google dot com>, Xinliang David Li <davidxl at google dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 11 Nov 2013 16:23:09 +0100
- Subject: Re: [PATCH] decide edge's hotness when there is profile info
- Authentication-results: sourceware.org; auth=none
- References: <20131015210324 dot GA23980 at kam dot mff dot cuni dot cz> <CAAe5K+X8MzvUHdpGVG5jKpz35XpjnPLig--eaHWG9pCNFdS3Bw at mail dot gmail dot com> <20131015215741 dot GA22365 at kam dot mff dot cuni dot cz> <CAAe5K+V+Yidij=WZM=rwfqfewuh81UOcUvHKVwT_g-9YwpUs-A at mail dot gmail dot com> <CAAe5K+Vw8YbvEe+qSKF8js_TgCEehdJ-qKVD2HkFS0pBRF+e+g at mail dot gmail dot com> <CAAe5K+XHKWOkZEaavJkFEfhWME4qKg_ByuXSrzxhKjgCu-QpWQ at mail dot gmail dot com> <20131018211757 dot GB31793 at kam dot mff dot cuni dot cz> <CAAe5K+XKp_5eBhEXxF29OmdrRv9nyr+aXW+PCdGH4hsFGY0X3w at mail dot gmail dot com> <CAAe5K+WZ2=YavTXf3yzdRHOmO9rEhvUM6xpDUKiRhkGhbX6JBQ at mail dot gmail dot com> <CAAe5K+UOn4wxvwsGhy_EjiFFQMDB-emRbpNJkY5bBf6QgRJo4w at mail dot gmail dot com>
> 2013-11-08 Teresa Johnson <tejohnson@google.com>
> Jan Hubicka <jh@suse.cz>
>
> * predict.c (drop_profile): New function.
> (handle_missing_profiles): Ditto.
> (counts_to_freqs): Don't overwrite estimated frequencies
> when function has no profile counts.
> * predict.h (handle_missing_profiles): Declare.
> * tree-inline.c (freqs_to_counts): New function.
> (copy_cfg_body): Invoke freqs_to_counts as needed.
> * tree-profile.c (tree_profiling): Invoke handle_missing_profiles.
>
> Index: predict.c
> ===================================================================
> --- predict.c (revision 204516)
> +++ predict.c (working copy)
> @@ -2765,6 +2765,107 @@ estimate_loops (void)
> BITMAP_FREE (tovisit);
> }
>
> +/* Drop the profile for NODE to guessed, and update its frequency based on
> + whether it is expected to be HOT. */
> +
> +static void
> +drop_profile (struct cgraph_node *node, bool hot)
> +{
> + struct function *fn = DECL_STRUCT_FUNCTION (node->decl);
> +
> + if (dump_file)
> + fprintf (dump_file,
> + "Dropping 0 profile for %s/%i. %s based on calls.\n",
> + cgraph_node_name (node), node->order,
> + hot ? "Function is hot" : "Function is normal");
> + /* We only expect to miss profiles for functions that are reached
> + via non-zero call edges in cases where the function may have
> + been linked from another module or library (COMDATs and extern
> + templates). See the comments below for handle_missing_profiles. */
> + if (!DECL_COMDAT (node->decl) && !DECL_EXTERNAL (node->decl))
> + warning (0,
> + "Missing counts for called function %s/%i",
> + cgraph_node_name (node), node->order);
> +
> + profile_status_for_function (fn)
> + = (flag_guess_branch_prob ? PROFILE_GUESSED : PROFILE_ABSENT);
> + node->frequency
> + = hot ? NODE_FREQUENCY_HOT : NODE_FREQUENCY_NORMAL;
> +}
> +
> +/* In the case of COMDAT routines, multiple object files will contain the same
> + function and the linker will select one for the binary. In that case
> + all the other copies from the profile instrument binary will be missing
> + profile counts. Look for cases where this happened, due to non-zero
> + call counts going to 0-count functions, and drop the profile to guessed
> + so that we can use the estimated probabilities and avoid optimizing only
> + for size.
> +
> + The other case where the profile may be missing is when the routine
> + is not going to be emitted to the object file, e.g. for "extern template"
> + class methods. Those will be marked DECL_EXTERNAL. Emit a warning in
> + all other cases of non-zero calls to 0-count functions. */
> +
> +void
> +handle_missing_profiles (void)
> +{
> + struct cgraph_node *node;
> + int unlikely_count_fraction = PARAM_VALUE (UNLIKELY_BB_COUNT_FRACTION);
> + vec<struct cgraph_node *> worklist;
> + worklist.create (64);
> +
> + /* See if 0 count function has non-0 count callers. In this case we
> + lost some profile. Drop its function profile to PROFILE_GUESSED. */
> + FOR_EACH_DEFINED_FUNCTION (node)
> + {
> + struct cgraph_edge *e;
> + gcov_type call_count = 0;
> + struct function *fn = DECL_STRUCT_FUNCTION (node->decl);
> +
> + if (node->count)
> + continue;
> + for (e = node->callers; e; e = e->next_caller)
> + call_count += e->count;
> + if (call_count
> + && fn && fn->cfg
> + && (call_count * unlikely_count_fraction >= profile_info->runs))
> + {
> + bool maybe_hot = maybe_hot_count_p (NULL, call_count);
> +
> + drop_profile (node, maybe_hot);
> + worklist.safe_push (node);
> + }
Perhaps we should add an note/error on mishandled profile when function is not COMDAT?
That way we may notice further bugs in this area.
> + }
> +
> + /* Propagate the profile dropping to other 0-count COMDATs that are
> + potentially called by COMDATs we already dropped the profile on. */
> + while (worklist.length () > 0)
> + {
> + struct cgraph_edge *e;
> +
> + node = worklist.pop ();
> + for (e = node->callees; e; e = e->next_caller)
> + {
> + struct cgraph_node *callee = e->callee;
> + struct function *fn = DECL_STRUCT_FUNCTION (callee->decl);
> +
> + if (callee->count > 0)
> + continue;
> + if (DECL_COMDAT (callee->decl) && fn && fn->cfg
> + && profile_status_for_function (fn) == PROFILE_READ)
Perhaps we can check here maybe_hot_bb_p for the caller and rely on profile estimate
to give us false only for really known to be unlikely paths? (i.e. EH handling, noreturns
etc.)
> + {
> + /* Since there are no non-0 call counts to this function,
> + we don't know for sure whether it is hot. Indicate to
> + the drop_profile routine that function should be marked
> + normal, rather than hot. */
> + drop_profile (node, false);
> + worklist.safe_push (callee);
> + }
> + }
> + }
> + worklist.release ();
I would add a pointer set so we avoid duplicates in worklist. This is potentially quadratic
for large programs.
OK, with these changes.
Honza