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 17:22:27 +0100
- Subject: Re: [PATCH] decide edge's hotness when there is profile info
- Authentication-results: sourceware.org; auth=none
- References: <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> <20131111152309 dot GC4119 at atrey dot karlin dot mff dot cuni dot cz> <CAAe5K+UgOsGOA7AfRRPVyj3nb-fYGu4-gFkjWBY1fcu1PWsR-w at mail dot gmail dot com>
> I have a warning like that already in drop_profile(). Is that
I think it should be warning (or silent) for COMDAT and error/note for
other functions (depending on flag_profile_correction).
I guess drop_profile is better place for it indeed.
> sufficient? Also, Steven Bosscher suggested putting that warning under
> OPT_Wdisabled_optimization instead of '0', what do you prefer for
> that?
It is a bit different case than other disabled optimizations we have (where
the optimization does not happen because of --param tunable limits), but I
am fine with both alternatives.
The warning may end up quite noisy so having way to silence it definitely
makes sense.
>
> >> + }
> >> +
> >> + /* 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.)
>
> Ok, let me try this.
>
> >> + {
> >> + /* 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.
>
> I'll add a visited_nodes set to keep track of processed nodes so they
> don't get added to the worklist multiple times.
Perhaps you can also track this by testing profile_status_for_function. Both solutions are fine for me.
Honza
>
> Thanks,
> Teresa
>
> >
> > OK, with these changes.
> >
> > Honza
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413