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.
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
