On Tue, Nov 12, 2013 at 11:08 AM, Teresa Johnson <tejohn...@google.com> wrote:
> On Mon, Nov 11, 2013 at 9:17 AM, Teresa Johnson <tejohn...@google.com> wrote:
>> On Mon, Nov 11, 2013 at 9:13 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>>>> On Mon, Nov 11, 2013 at 8:22 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>>>> >> I have a warning like that already in drop_profile(). Is that
>>>> >
>>>> > I think it should be warning (or silent) for COMDAT and error/note for
>>>> > other functions (depending on flag_profile_correction).
>>>> > I guess drop_profile is better place for it indeed.
>>>>
>>>> I don't want to warn for COMDAT since this is currently an expected
>>>> issue in that case, so I'm afraid it will be too noisy (but there is a
>>>> note emitted to the dump file to track how often this occurs). So
>>>> currently the warning is only emitted in cases where we don't expect
>>>> it to occur (e.g. non-comdat).
>>>
>>> I see, I misread it.
>>> The non-comdat warning IMO ought go the same way as the other profile
>>> confussions.
>>> I guess something like
>>> if (!flag_profile_correction)
>>> error (...);
>>> like existing cases in profile.c
>>
>> Ok, will do (emitting a note to the dump file as in other cases in
>> profile.c that do this same thing).
>>
>>>>
>>>> I didn't find any warnings being emitted when running this for spec or
>>>> internal benchmarks, so how about instead of a warning, I handle it
>>>> like you suggest above: depending on the setting of
>>>> flag_profile_correction either error or emit a note to the dump file
>>>> under MSG_MISSED_OPTIMIZATION?
>>>
>>> Yep, lets just go with error with !flag_profile_correction and being silent
>>> otherwise.
>>> If we want to go with warnings, we need to change all the similar places
>>> in profile.c and elsewhere.
>>>>
>>>> Ah - I just realized I am already checking profile_status_for_function
>>>> above and adding to the worklist if it is still PROFILE_READ. Since I
>>>> call drop_profile before adding to the worklist, we can't end up
>>>> adding multiple times. So I don't think there is currently an issue
>>>> with this.
>>>
>>> Yep, indeed.
>>>
>>> The patch is OK with the error message as discussed above.
>>
>> Ok, will do that and fix a couple of minor issues mentioned by Steven
>> (missing comment and incorrect indentation in one spot). Will retest
>> just to make sure I didn't miss any warnings which will now be errors.
>>
>> Thanks,
>> Teresa
>>
>>>
>>> Thanks.
>>> Honza
>>>>
>>>> Thanks,
>>>> Teresa
>>>>
>>>> >
>>>> > Honza
>>>> >>
>>>> >> Thanks,
>>>> >> Teresa
>>>> >>
>>>> >> >
>>>> >> > OK, with these changes.
>>>> >> >
>>>> >> > Honza
>
> This was committed earlier this morning as r204704. Then I hit the new
> error when doing an lto-bootstrap profiledbootstrap for an unrelated
> change. I'd like to change this error to a warning for now to avoid
> breaking the lto profiledbootstrap while I investigate. I don't think
> this really needs to be a hard error at this point.
More info on the lto bootstrap failure:
/usr/local/google/home/tejohnson/gcc_trunk_9/libiberty/pex-unix.c:790:1:
warning: Missing counts for called function pex_child_error.isra.1/73
[enabled by default]
}
This is an error handling routine that invokes _exit. Looking at the
ipa-profile dump, I can see that all the counts read in for
pex_child_error have the value 0. But there is a non-zero callsite,
that not surprisingly complains of a profile insanity in the bb's
outgoing edge weights, since pex_child_error never returns:
;; basic block 38, loop depth 1, count 192, freq 5000
;; Invalid sum of outgoing counts 0, should be 192
...
pex_child_error.isra.1D.5005 (_92, executable_40(D),
[/usr/local/google/home/tejohnson/gcc_trunk_9/libiberty/pex-unix.c :
677] "execv", _91);
;; succ: 3 [100.0%] (ABNORMAL,DFS_BACK,IRREDUCIBLE_LOOP,EXECUTABLE)
Not sure why the counts were all 0 for this routine though, I wouldn't
think the early exit should prevent us from updating the counts. But
IMO the best thing to do here is to issue a warning, since I don't
want more issues like this to cause compile errors when we handled
them fine before.
Let me know if the patch below is ok.
Thanks,
Teresa
>
> Running regression tests and lto profiledbootstrap right now. Ok for
> trunk if testing succeeds?
>
> 2013-11-12 Teresa Johnson <tejohn...@google.com>
>
> * predict.c (drop_profile): Error is currently too strict.
>
> Index: predict.c
> ===================================================================
> --- predict.c (revision 204704)
> +++ predict.c (working copy)
> @@ -2792,8 +2792,8 @@ drop_profile (struct cgraph_node *node, bool hot)
> cgraph_node_name (node), node->order);
> }
> else
> - error ("Missing counts for called function %s/%i",
> - cgraph_node_name (node), node->order);
> + warning (0, "Missing counts for called function %s/%i",
> + cgraph_node_name (node), node->order);
> }
>
> profile_status_for_function (fn)
>
> Thanks,
> Teresa
>
>>>> >>
>>>> >>
>>>> >>
>>>> >> --
>>>> >> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>>>>
>>>>
>>>>
>>>> --
>>>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
--
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
