The patch is ok.
David
On Thu, Oct 10, 2013 at 8:09 AM, Dehao Chen <de...@google.com> wrote:
> Patch updated.
>
> Thanks,
> Dehao
>
> On Wed, Oct 9, 2013 at 10:45 PM, Xinliang David Li <davi...@google.com> wrote:
>> > /* 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
>> iterations.
>>
>>> 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))
>>> break;
>>> 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?
>
> That's still needed because the IC_promotion happened before
> annotation is only for those promoted in the profiling runs. We still
> need to promote those newly exposed opportunities.
>
>>
>> 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.
>
> Those callsites will still be promoted after annotation, thus no problem.
>
> Dehao
