This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] decide edge's hotness when there is profile info
- From: Teresa Johnson <tejohnson at google dot com>
- To: Jan Hubicka <hubicka at ucw dot cz>
- Cc: 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: Tue, 15 Oct 2013 15:39:21 -0700
- Subject: Re: [PATCH] decide edge's hotness when there is profile info
- Authentication-results: sourceware.org; auth=none
- References: <CAAkRFZJWSXd6wkVXwsvPf_zkjY=5DF0YhRL6LCnkPn1qttebUw at mail dot gmail dot com> <CAO2gOZVPv8G5yy6K+itKN=Ra6JCUkpZd2HGGfb34ZKkP75pmuA at mail dot gmail dot com> <20131014215027 dot GD17422 at kam dot mff dot cuni dot cz> <CAO2gOZXeGP8nZsa0aoji1dVxkRjT69G_ySU3iWbK9oJrjF2n_g at mail dot gmail dot com> <CAAe5K+VJ70+Sj_J-uwcr+W6THPZXe5cy-hwtBCpJXms2=nPimg at mail dot gmail dot com> <CAO2gOZVmMLBEnEYdh4yFD8D5y0netp8V0VLNW1Wcwt9fcwEYkw at mail dot gmail dot com> <CAAe5K+V5k2i9gpo0Ak8b5MnC87uSeaJWJzkRUrZxtoF1M8crNg at mail dot gmail dot com> <CAO2gOZV-opRL_R-1sVQD8BdMaUDHac3H+WfF8Y-vkdHzRd__Sg at mail dot gmail dot com> <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>
On Tue, Oct 15, 2013 at 2:57 PM, Jan Hubicka <email@example.com> wrote:
>> On Tue, Oct 15, 2013 at 2:03 PM, Jan Hubicka <firstname.lastname@example.org> wrote:
>> >> Yes, that would work. So let's discard this patch because the fix for
>> >> comdat can also fix this problem.
>> > Unforutnately ipa-profile-estimate is an IPA pass and as such it does
>> > not have access to profile_status_to_function.
>> I see. I was coding that up today but hadn't tested it yet.
>> > You can probably just factor this out into a function that can be called
>> > and for normal FDO we call it where the loop stis now and for auto-FDO we can
>> > probably have another invocation from before early passes where auto-FDO is collected.
>> Ok, let's go with that approach for now. It won't address the 0 count
>> COMDAT calling another 0 count COMDAT problem, but I will probably
>> just find a way to deal with this when inlining.
> You can still propagate, since tree-profile is an simple-ipa pass.
Ok, I think I will leave that for the second patch, since I need to do
some testing on the effects - i.e. the propagation I had in mind would
make any 0-count routine called by a routine that was dropped to
guessed to also be dropped to guessed (and no longer unlikely). It may
be too aggressive, I need to check.
>> >> >>> + if (node->count)
>> >> >>> + continue;
>> > Also here we should sum the counts and consider function non unlikely executed
>> > in the same way as probably_never_executed does.
>> I assume you mean by doing the same comparison to the number of
>> profile->runs. Yes, this makes sense.
>> > I can prepare updated patch, but i am currently travelling, so i would not
>> > be disapointed if you beat me ;)
>> I'm working on it, and I think based on Dehao's needs I am going to
>> split up the patch into two phases, the one that does just the part
>> you had sent a patch for (making sure 0 count routines with non-zero
>> calls are marked guessed and have their node frequency set
>> appropriately), and a subsequent one to do the count application when
>> we inline a 0-count routine into a non-zero callsite. I'll shoot for
>> getting this ready by tomorrow.
>> BTW, in your original patch you are checking for both e->count or
>> cgraph_maybe_hot_edge_p(e), but AFAICT the call to
>> cgraph_maybe_hot_edge_p will never return true when e->count is zero.
>> When there is a profile it will return false via maybe_hot_count_p
>> since e->count == 0. When there is no profile it will return false
>> when the callee has NODE_FREQUENCY_UNLIKELY_EXECUTED. So I think just
>> checking for e->count >0 is sufficient here.
> I think I was checking caller count here (that is read) and the code
> was supposed to make functoin with hot caller to be hot...
The code in your patch was just checking the edge count, not the
caller count. cgraph_maybe_hot_edge_p also doesn't check the caller
count, just the edge count. Do we want to make all functions with hot
callers also be hot, even when the edge count is not hot? This may be
to aggressive. I was simply going to make the code check e->count.
>> > Honza
>> Teresa Johnson | Software Engineer | email@example.com | 408-460-2413
Teresa Johnson | Software Engineer | firstname.lastname@example.org | 408-460-2413