From: Teresa Johnson <tejohnson@google.com>
To: Jan Hubicka <hubicka@ucw.cz>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
"marxin.liska" <marxin.liska@gmail.com>
Subject: Re: [PATCH] Sanitize block partitioning under -freorder-blocks-and-partition
Date: Fri, 27 Sep 2013 14:50:00 -0000 [thread overview]
Message-ID: <CAAe5K+XEmfA7gwZrBgFALtXMwAmbn0XPdF_CtMA1uEqXq5oyRQ@mail.gmail.com> (raw)
In-Reply-To: <20130926220209.GB13383@kam.mff.cuni.cz>
On Thu, Sep 26, 2013 at 3:02 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>
>> Why not just have probably_never_executed_bb_p return simply return
>> false bb->frequency is non-zero (right now it does the opposite -
>
> We want to have frequencies guessed for functions that was not trained
> in the profiling run (that was patch I posted earlier that I think did not
> go in, yet).
Right, but for splitting and bb layout purposes, for these statically
guessed unprofiled functions we in fact don't want to do any splitting
or treat the bbs as never executed (which shouldn't be a change from
the status quo since all the bbs in these functions are currently 0
weight, it's only when we inline in the case of comdats that they
appear colder than the surrounding code, but in fact we don't want
this).
The only other caller to probably_never_executed_bb_p is
compute_function_frequency, but in the case of statically guessed
functions they will have profile_status != PROFILE_READ and won't
invoke probably_never_executed_bb_p. But re-reading our most recent
exchange on the comdat profile issue, it sounds like you were
suggesting guessing profiles for all 0-weight functions early, then
dropping them from PROFILE_READ to PROFILE_GUESSED only once we
determine in ipa-inline that there is a potentially non-zero call path
to them. In that case with the change I describe above to
probably_never_executed_bb_p, the 0-weight functions with 0 calls to
them will incorrectly be marked as NODE_FREQUENCY_NORMAL, which would
be bad as they would not be size optimized or moved into the cold
section.
So it seems like we want different handling of these guessed
frequencies in compute_function_frequency and bb-reorder.c. Actually I
think we can handle this by checking if the function entry block has a
0 count. If so, then we just look at the bb counts and not the
frequencies for determining bb hotness as the frequencies would
presumably have been statically-guessed. This will ensure that the
cgraph node continues to be marked unlikely and size-optimized. If the
function entry block has a non-zero count, then we look at both the bb
count and the bb frequency - if they are both zero then the bb is
probably never executed, but if either is non-zero then we should
treat the block as possibly executed (which will come into play for
splitting and bb layout).
Teresa
>
> Currently I return true when frequency indicate that BB is executed at least in
> 1/4th of all executions. With the cases discussed I see we may need to reduce
> this threshold. In general I do not like much hard tests for 0 because meaning
> of 0 depends on REG_BR_FREQ_BASE that is supposed to be changeable and we may
> want to make frequencies sreal, too.
>
> I suppose we may introduce --param for this. You are also right that I should
> update probably_never_executed_edge_p (I intended so, but obviously the code
> ended up in mainline accidentally).
>
> I however saw at least one case of jump threading where this trick did not
> help: the jump threading update confused itself by scaling via counts rather
> than frequencies and ended up with dropping everything to 0. This makes it
> more tempting to try to go with sreals for those....
>
> Honza
>
>> returns true when bb->frequency is 0)? Making this change removed a
>> bunch of other failures. With this change as well, there are only 3
>> cases that still fail with 1 train run that pass with 100. Need to
>> look at those.
>>
>> >
>> > Will you look into logic of do_jump or shall I try to dive in?
>>
>> I can take a look, but probably won't have a chance until late this
>> week. If you don't get to it before then I will see if I can figure
>> out why it is applying the branch probabilities this way.
>>
>> Teresa
>>
>> >
>> > Honza
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
--
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
next prev parent reply other threads:[~2013-09-27 14:15 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-01 16:32 Teresa Johnson
2013-08-02 11:22 ` Bernhard Reutner-Fischer
2013-08-02 14:51 ` Teresa Johnson
2013-08-02 15:05 ` Jan Hubicka
2013-08-02 23:05 ` Steven Bosscher
2013-08-03 4:53 ` Teresa Johnson
2013-08-03 4:48 ` Teresa Johnson
2013-08-05 13:36 ` Teresa Johnson
2013-08-05 14:11 ` Jan Hubicka
2013-08-05 14:57 ` Teresa Johnson
2013-08-06 3:01 ` Teresa Johnson
[not found] ` <20130808222332.GA31755@kam.mff.cuni.cz>
2013-08-08 23:04 ` Teresa Johnson
2013-08-09 9:58 ` Jan Hubicka
2013-08-09 14:38 ` Teresa Johnson
2013-08-09 15:28 ` Jan Hubicka
2013-08-09 15:54 ` Martin Liška
2013-08-09 21:03 ` Teresa Johnson
2013-08-09 21:02 ` Teresa Johnson
2013-08-09 22:43 ` Jan Hubicka
2013-08-11 12:21 ` Jan Hubicka
2013-08-11 13:25 ` Teresa Johnson
2013-08-11 15:55 ` Martin Liška
2013-08-11 17:55 ` Jan Hubicka
2013-08-11 21:05 ` Jan Hubicka
2013-08-17 15:54 ` Teresa Johnson
2013-08-17 21:02 ` Jan Hubicka
2013-08-19 13:51 ` Teresa Johnson
2013-08-19 15:16 ` Jan Hubicka
2013-08-19 17:48 ` Teresa Johnson
2013-08-19 19:56 ` Martin Liška
2013-08-27 18:12 ` Teresa Johnson
2013-08-28 16:59 ` Jan Hubicka
2013-08-28 18:35 ` Teresa Johnson
2013-08-30 7:17 ` Teresa Johnson
2013-08-30 9:16 ` Jan Hubicka
2013-08-30 15:13 ` Teresa Johnson
2013-08-30 15:28 ` Jan Hubicka
2013-08-30 15:54 ` Teresa Johnson
2013-08-30 21:56 ` Rong Xu
2013-08-31 16:20 ` Jan Hubicka
2013-08-31 23:40 ` Jan Hubicka
2013-09-24 8:07 ` Teresa Johnson
2013-09-24 13:44 ` Jan Hubicka
2013-09-24 19:06 ` Teresa Johnson
2013-09-26 20:55 ` Rong Xu
2013-09-26 22:23 ` Jan Hubicka
2013-09-26 22:54 ` Rong Xu
2013-09-24 18:28 ` Jan Hubicka
2013-09-24 18:51 ` Teresa Johnson
2013-09-25 23:10 ` Teresa Johnson
2013-09-26 8:44 ` Teresa Johnson
2013-09-26 22:26 ` Jan Hubicka
2013-09-27 14:50 ` Teresa Johnson [this message]
2013-09-29 17:34 ` Teresa Johnson
2013-10-02 16:19 ` Jan Hubicka
2013-10-02 17:55 ` Teresa Johnson
2013-10-02 18:10 ` Jan Hubicka
2013-10-03 13:42 ` Teresa Johnson
2013-10-03 23:37 ` Teresa Johnson
2013-10-01 17:36 ` Teresa Johnson
2013-08-19 15:34 ` Teresa Johnson
2013-08-21 15:31 ` Jan Hubicka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAAe5K+XEmfA7gwZrBgFALtXMwAmbn0XPdF_CtMA1uEqXq5oyRQ@mail.gmail.com \
--to=tejohnson@google.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hubicka@ucw.cz \
--cc=marxin.liska@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).