Re: FDO usability patch -- correct insane profile data

[Xinliang David Li <davidxl at google dot com>]

Adding assertion sounds good to me -- the only problem is that problem
like this hard to be triggered during testing, thus making assertion
less useful as it can be. Is it better to add the assertion with
checking is enabled while still keeping the correction code?

Thanks,

David

On Sun, Apr 17, 2011 at 11:53 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> hmm, looks we both run into same issue and developed different fixes.
>>
>> Good point. The change above was added based on 4.4.3 where the sum
>> computation has no capping like above. Yes, with the current capping,
>> the negative value won't result. However, it is still good to guard
>> the scale independent of the implementation of ipcp_compute_node_scale
>> -- it may change and break silently (the comment of the function says
>> the implementation is wrong...)
>
> Well, if we want to add check in anticipation that we will introduce bug few
> lines up, I think we could just add gcc_assert (scale < BASE); with an comment
> explaining that ipcp_compute_node_scale must give results in range even when
> profile is not flow consistent.
>
> Since we need to propagate the scales across callgraph (at least for the
> correct implementation of the function), masking bug in ipcp_compute_node_scale
> would make us to propagate the nonsenses further and silently producing
> unnecesarily aplified insanity.
>
> I would pre-approve patch reverting the current change and adding the assert
> with comment.
>
> Honza
>>
>> Thanks,
>>
>> David
>>
>>
>> >
>> > Honza
>> >
>