This is the mail archive of the
mailing list for the GCC project.
Re: [GOOGLE] Iterative AutoFDO
- From: Xinliang David Li <davidxl at google dot com>
- To: Dehao Chen <dehao at google dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 9 Oct 2013 22:45:25 -0700
- Subject: Re: [GOOGLE] Iterative AutoFDO
- Authentication-results: sourceware.org; auth=none
- References: <CAO2gOZX+xnp_0JFg3CaS5Z_=KU78Hd7BGRoxcE+RgEXY_R--dQ at mail dot gmail dot com> <CAAkRFZKFLOA0dDE9ca5sCAiKNCQaX=MFhq71Q=V0FRpdkKtiUA at mail dot gmail dot com> <CAO2gOZWApmZO009dzjQQit1+FrR4EXO5iyYqUf4s7ZX1nLzS2Q at mail dot gmail dot com>
> /* Program behavior changed, original promoted (and inlined) target is not
> hot any more. Will avoid promote the original target. */
> if (total >= info->first * 0.5)
> return false;
This part is still not clear: Difference between OLD_INFO and INFO,
factor 0.5 comes from where etc.
> gcov_type autofdo_source_profile::get_callsite_total_count (
> struct cgraph_edge *edge) const
Formatting and missing documentation -- there are other cases like
this in the source.
> /* No need to promoted the stmt if its in promoted_stmts (means
> it is already been promoted in the previous iterations). */
Make it clear that ic promotion and early-inline-2 is done in multiple
> This is needed because an indirect call might be promoted and
might have been
> inlined in the profiled binary. If we do not promote and inline
> these indirect calls before annothation, the profile for these
annothation --> annotation.
> for (int i = 0; i < PARAM_VALUE (PARAM_EARLY_INLINER_MAX_ITERATIONS); i++)
> if (!flag_value_profile_transformations
> || !autofdo::afdo_vpt_for_early_inline (&promoted_stmts))
> early_inliner ();
Does it leave any indirect call sites not promoted after this loop? In
other words, is there a need to keep the value profile transformation
after the cfg annotation?
A related question -- for indirect callsites that are not
promoted/inlined in the profiled binary, will indirect call promotion
and inlining before cfg annotation cause problems? The inline instance
won't find profile anymore.
On Mon, Oct 7, 2013 at 11:49 AM, Dehao Chen <firstname.lastname@example.org> wrote:
> Hi, David,
> Thanks for the review. Patch updated.
> On Sun, Oct 6, 2013 at 11:54 AM, Xinliang David Li <email@example.com> wrote:
>> Adding additional early inline + value transform (calling them as
>> utility functions) is 'unconventional' way of invoking passes. It
>> would be helpful to do more heavy documentation by providing more
>> examples and explaining why simply augmenting the indirect target info
>> for promoted icall site with inlined call stack profile data and rely
>> on existing value profile transform to kick in is not enough.
>> The patch also misses documentation for many functions at the
>> definition site. There are also a couple of coding style problems such
>> as function name does not start a new line (but following return
>>> /* Return the combined relative location for STMT. */
>>> static unsigned
>>> get_relative_location_for_stmt (gimple stmt)
>> More explanation on 'relative' -- it is not mentioned in other places either.
>>> /* Program behavior changed, original promoted (and inlined) target is not
>>> hot any more. Will avoid promote the original target. */
>>> if (total >= info->first * 0.5)
>>> return false;
>> This needs more explanation.
>>> /* For a given BB, return its execution count, and annotate value profile
>>> if a stmt is not in PROMOTED. Add the location of annotated stmt to
>>> ANNOTATED. */
>> More explanation on why skipping promoted ones.
>> > || promoted_stmts->find (stmt) != promoted_stmts->end ())
>> > continue;
>> > promoted_stmts->insert (stmt);
>> It is not quite clear how 'promoted' stmts are identified here.
>>> /* First do indirect call promotion and early inline to make the
>>> IR match the profiled binary before actual annotation. */
>>> autofdo::stmt_set promoted_stmts;
>>> for (int i = 0; i < PARAM_VALUE (PARAM_EARLY_INLINER_MAX_ITERATIONS); i++)
>>> early_inliner ();
>>> if (!flag_value_profile_transformations
>>> || !autofdo::afdo_vpt_for_early_inline (&promoted_stmts))
>> This needs heavy documentation. ALso why putting early inline before
>> value transform?
>> On Mon, Sep 23, 2013 at 10:51 AM, Dehao Chen <firstname.lastname@example.org> wrote:
>>> This patch fixes the issue of indirect call promotion while using
>>> AutoFDO optimized binary to collect profile, and use the new profile
>>> to re-optimize the binary. Before the patch, the indirect call is
>>> promoted (and likely inlined) in the profiled binary, and will not be
>>> promoted in the new iteration. With the patch, a new iterative
>>> vpt+einline pass is added before annotation to make sure hot promoted
>>> indirect calls are still promoted before annotation.
>>> Bootstrapped and passed regression test and benchmark test.
>>> OK for google-4_8 branch?