On 23 January 2016 at 23:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Noah Misch <noah@leadboat.com> writes:
>> On Sat, Jan 23, 2016 at 05:04:56PM -0500, Tom Lane wrote:
>>> Either I missed something or there's another issue, because tern/sungazer
>>> are *still* failing. This is getting annoying :-(
>
>> sungazer's "make check" passes if I change init_degree_constants() to be
>> non-static. Duping gcc isn't so easy these days.
>
> Ugh. Well, at least we don't have to move it to another file, which was
> going to be my next larger size of hammer.
>
> Thanks for doing the legwork on this!
>
Hi, I'm just now catching up on my email after being out of town and
not reading it. Thanks for looking at this and sorting out those
issues, and thank you also Noah and Peter for your investigative work.
If I understand correctly there were 2 separate issues at play here:
1). On some platforms the compiler evaluates expressions like
sin(constant) and comes up with a slightly different result than a
runtime evaluation of the expression. The compiler-evaluated result is
presumably a 64-bit IEEE float, but at runtime it may be using
extended precision for intermediate results. That may well have been
the sole contributing factor to the fact that sind(30) wasn't exactly
0.5.
2). The compiler also sometimes rearranges expressions in ways that
don't give the same result as evaluating in the order suggested by the
parentheses. I think this actually explains the failure to get exactly
1 for tand(45). For x=45, this was being computed as
cosd_0_to_60(90 - x) / cosd_0_to_60(x)
so my guess is that it was inlining cosd_0_to_60(90 - x) and
rearranging it to produce something different from cosd_0_to_60(x) for
x=45.
Looking at the new code, it's annoying how much effort was needed to
prevent the compiler messing it up. I ought to have realised that the
optimiser would be awkward for this kind of thing.
I wonder if the same could have been achieved by disabling
optimisation and inlining in those low-level functions, and also
wrapping sin(x * RADIANS_PER_DEGREE) in a similar non-inlinable,
non-optimised function to force it to be executed at runtime when
passed a constant. That would probably have made them significantly
slower though, whereas the new code benefits from various pre-computed
expressions.
Thanks again for fixing this.
Regards,
Dean