On Thu, Nov 8, 2018 at 6:33 AM Jeff Law <l...@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.ch...@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.ch...@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.ch...@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.
0003-Fix-AutoFDO-breakage-after-profile-count-rewriting.patch
Description: Binary data
