Re: [PATCH] Change the badness computation to ensure no integer-underflow

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Aug 28, 2013 at 2:13 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Tue, Aug 27, 2013 at 2:50 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >> >>
>> >> >> I've updated the patch (as attached) to use sreal to compute badness.
>> >> >>>> +      badness = ((int)((double) edge->count / max_count
>> >> >>>> +  * relbenefit / RELATIVE_TIME_BENEFIT_RANGE * INT_MIN / 2)) / growth;
>> >> >>>> +
>> >> >>>
>> >> >>> FP operations on the host are frowned upon if code generation depends
>> >> >>> on their outcome.  They all should use sreal_* as predict already does.
>> >> >>> Other than that I wonder why the final division is int (so we get truncation
>> >> >>> and not rounding)?  That increases the effect of doing the multiply by
>> >> >>> relbenefit in double.
>> > Sorry, i missed this patch - I need to update my scripts marking files interesting
>> > to me.
>> >
>> > I originally duplicated the trick from global.c that used to do this for ages.
>> > The rationale is that if you do only one FP operation, then you will not get
>> > difference in between hosts doing 80bit fp (x86) and 64bit fp (others) and you
>> > do the operation a lot faster than sreal does. While this is on slipperly ground,
>> > controlled use of double should not imply host dependency.
>>
>> Well .. the above is clearly not "one" FP operation.  Also I don't follow.
>> If we can assume all hosts implement proper IEEE double math then
>> of course the issue is moot.  But fact is we don't (not sure if we can
>
> We are not really consistent across all hosts either, because of HOST_WIDE_INT
> differences.  What we really cared about is to have compiler that does not give
> different result based on build flags.

Well, we care that a cross from X to T produces the same code as a cross
from Y to T or a native compiler on T.  Otherwise it's hard to even think of
using icecream like we do internally.

>> declare such hosts broken and unsupported as host).  Of couse on
>> a perfect IEEE compliant host sreal.[ch] could be simply mapped to
>> double.  Oh, and yes, 80bit x87 unit isn't holding up to that premises.
>
> sreal is not IEEE compliant.  We first time hit the problem of doing
> non-trivial chains of fp opreations in predict.c during my school project and
> Josef implemented it as a dirty but resonably quick FP emulation module.
> Hopefully it will work well enough in badness computationm, too.
>
> I am giving the patch brief benchmarking on profiledbootstrap and it it won't
> cause major regression, I think we should go ahead with the patch.
>
> I was never really happy about the double use there and in fact the whole fixed
> point arithmetic in badness compuation is a mess.  If we had template based
> fibonaci heap and sreal fast enough, turing it all to reals would save quite
> some maintenance burden.

Yeah, well.

Richard.

> Honza