This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Ensure count_scale is no larger than REG_BR_PROB_BASE
- From: Dehao Chen <dehao at google dot com>
- To: Jan Hubicka <hubicka at ucw dot cz>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, David Li <davidxl at google dot com>
- Date: Mon, 19 May 2014 14:15:24 -0700
- Subject: Re: [PATCH] Ensure count_scale is no larger than REG_BR_PROB_BASE
- Authentication-results: sourceware.org; auth=none
- References: <CAO2gOZVj7vwDDtn1=XjE4PsiB-1B-a-9Ku2gFHViOVOLZUYMrg at mail dot gmail dot com> <20140516234137 dot GC28043 at kam dot mff dot cuni dot cz> <CAO2gOZUx92=GXQ-7ouET5zk+DCZbw=OPFTco=wPYhy-n-TofcQ at mail dot gmail dot com> <20140517002207 dot GD28043 at kam dot mff dot cuni dot cz> <CAO2gOZWRRkKOyQ8=FkyNyrVfp02XJDiqdedbUVKAajvsRPJ5dw at mail dot gmail dot com> <20140517014113 dot GA10844 at kam dot mff dot cuni dot cz> <CAO2gOZVsnuEsNqJKMMj=cxbB4H6WB9o3j+twy=Ji2_poEz-Jow at mail dot gmail dot com> <20140519204049 dot GB13429 at kam dot mff dot cuni dot cz>
On Mon, May 19, 2014 at 1:40 PM, Jan Hubicka <email@example.com> wrote:
>> I've updated the patch. Shall I move the check inside cgraph_clone_node?
> I think it is OK as it is. I belive individual users should know what do to
> in such cases themselves.
> You may want to also check what ipa-cp is doing.
I checked ipa-cp, but didn't see count propagation anywhere. Could you
point me to the function?
> Patch is OK (with Changelog)
>> Index: gcc/ipa-inline-transform.c
>> --- gcc/ipa-inline-transform.c (revision 210535)
>> +++ gcc/ipa-inline-transform.c (working copy)
>> @@ -183,8 +183,9 @@ clone_inlined_nodes (struct cgraph_edge *e, bool d
>> if (freq_scale == -1)
>> freq_scale = e->frequency;
>> n = cgraph_clone_node (e->callee, e->callee->decl,
>> - e->count, freq_scale, update_original,
>> - vNULL, true, inlining_into, NULL);
>> + MIN (e->count, e->callee->count), freq_scale,
>> + update_original, vNULL, true, inlining_into,
>> + NULL);
>> cgraph_redirect_edge_callee (e, n);
>> Index: gcc/tree-inline.c
>> --- gcc/tree-inline.c (revision 210535)
>> +++ gcc/tree-inline.c (working copy)
>> @@ -4355,7 +4355,7 @@ expand_call_inline (basic_block bb, gimple stmt, c
>> function in any way before this point, as this CALL_EXPR may be
>> a self-referential call; if we're calling ourselves, we need to
>> duplicate our body before altering anything. */
>> - copy_body (id, bb->count,
>> + copy_body (id, cg_edge->callee->count,
>> GCOV_COMPUTE_SCALE (cg_edge->frequency, CGRAPH_FREQ_BASE),
>> bb, return_block, NULL);
>> On Fri, May 16, 2014 at 6:41 PM, Jan Hubicka <firstname.lastname@example.org> wrote:
>> >> Do you mean adjusting bb->count? Because in
>> >> expand_call_inline(tree-inline.c), it will use bb->count to pass into
>> >> copy_body to calculate count_scale.
>> > What about taking here callee->count instead? For inline nodes without
>> > any capping hack, bb->count == edge->count = callee->count.
>> > When profile ends up being obviously inconsistent, I would say that
>> > inliner can cap callee->count during producing the clone...
>> > Honza
>> >> Thanks,
>> >> Dehao
>> >> On Fri, May 16, 2014 at 5:22 PM, Jan Hubicka <email@example.com> wrote:
>> >> >> In AutoFDO, a basic block's count can be much larger than it's actual
>> >> >> count because debug info might be incorrect. In this case, a call edge
>> >> >> count (calculated from BB count) could be much larger than callee's
>> >> >> header count, making the count_scale incorrectly large.
>> >> >
>> >> > In this case I still think we should handle this when producing the clone:
>> >> > we do not want to have clone's count much larger as well, so i think inliner
>> >> > and ipa-cp needs to deal with capping here instead....
>> >> >
>> >> > Honza
>> >> >>
>> >> >> Dehao
>> >> >> >
>> >> >> >
>> >> >> > Honza