[PATCH AutoFDO/4]Fix profile count computation/propagation.

[Jeff Law]

On 12/10/18 6:36 PM, Bin.Cheng wrote:
> On Thu, Nov 8, 2018 at 6:33 AM Jeff Law <law@redhat.com> wrote:
>> On 10/31/18 12:34 AM, bin.cheng wrote:
>>> Hi,
>>> This patch fixes AutoFDO breakage on trunk.  The main reason for breakage is AutoFDO
>>> relies on standalone edge count computing and propagating profile count/probability info
>>> on CFG, but in new infra, edge count is actually computed from probability, which leads
>>> to chicken-egg problem and corrupted profile count.  This patch fixes the issue by using
>>> explicit edge count.
>>>
>>> There is another issue not touched yet that, in quite common case, profiled samples are
>>> not enough and profile info computed for lots of blocks is ZERO.  In the future, we may
>>> add some heuristics checking quality of sampled counts and reverting to guessed profile
>>> count if necessary.  I think change made in this patch is also needed for that.
>>>
>>> Package mysql server is used in test of this patch set.  It can't be compiled with autofdo
>>> on trunk, even with compilation issues worked-around, there isn't performance improvement.
>>> I local experiments, with this patch set it's improved by 12.3%, 4.3% irrespectively for
>>> read-only/write-heavy benchmarks.  Unfortunately,  this patch set was written against
>>> GCC 8 branch a while ago, improvement gets worse on trunk and I haven't investigated
>>> the reason yet.  I guess there are still other issues which need to be fixed in the future.
>>>
>>> Bootstrap and test on x86_64 in patch set.  Is it OK?
>>>
>>> Thanks,
>>> bin
>>> 2018-10-31  Bin Cheng  <bin.cheng@linux.alibaba.com>
>>>
>>>       * auto-profile.c (AFDO_EINFO): New macro.
>>>       (struct edge_info): New structure.
>>>       (is_edge_annotated, set_edge_annotated): Delete.
>>>       (afdo_propagate_edge, afdo_propagate_circuit, afdo_propagate): Remove
>>>       parameter.  Adjust edge count computation and annotation using struct
>>>       edge_info.
>>>       (afdo_calculate_branch_prob): Ditto.
>>>       (afdo_annotate_cfg): Simplify code setting basic block profile count.
>>>
>>>
>>> 0004-Fix-AutoFDO-breakage-after-profile-count-rewriting.patch
>>>
>>> From 6506c12d1b633b6d1bfae839b3633a4f99b3a481 Mon Sep 17 00:00:00 2001
>>> From: chengbin <bin.cheng@linux.alibaba.com>
>>> Date: Mon, 20 Aug 2018 15:25:02 +0800
>>> Subject: [PATCH 4/4] Fix AutoFDO breakage after profile count rewriting.
>>>
>>> ---
>>>  gcc/auto-profile.c | 190 ++++++++++++++++++++++++++---------------------------
>>>  1 file changed, 95 insertions(+), 95 deletions(-)
>>>
>>> diff --git a/gcc/auto-profile.c b/gcc/auto-profile.c
>>> index cde4f41c1d9..ff3ea23d830 100644
>>> --- a/gcc/auto-profile.c
>>> +++ b/gcc/auto-profile.c
>>> @@ -101,6 +101,17 @@ along with GCC; see the file COPYING3.  If not see
>>>  namespace autofdo
>>>  {
>>>
>>> +/* Intermediate edge info used when propagating AutoFDO profile information.
>>> +   We can't edge->count() directly since it's computed from edge's probability
>>> +   while probability is yet not decided during propagation.  */
>>> +#define AFDO_EINFO(e)                     ((struct edge_info *) e->aux)
>>> +struct edge_info
>>> +{
>>> +  edge_info () : count (profile_count::zero ().afdo ()), annotated_p (false) {}
>>> +  profile_count count;
>>> +  bool annotated_p;
>>> +};
>> edge_info isn't POD, so make it a class rather than a struct.
>>
>> OK with that change assuming it does not have a hard dependency on prior
>> patches in this series.
> Thanks very much for review.  Now that all prerequisite patches are
> approved and committed, I update this one by making edge_info a class
> as suggested.
> Bootstrap and test as before, is it OK?
> 
> Thanks,
> bin
> 
> 2018-12-11  Bin Cheng  <bin.cheng@linux.alibaba.com>
> 
>         * auto-profile.c (AFDO_EINFO): New macro.
>         (class edge_info): New class.
>         (is_edge_annotated, set_edge_annotated): Delete.
>         (afdo_propagate_edge, afdo_propagate_circuit, afdo_propagate): Remove
>         parameter.  Adjust edge count computation and annotation using class
>         edge_info.
>         (afdo_calculate_branch_prob, afdo_annotate_cfg): Likewise.
OK
jeff