From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
Cc: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [COMMITTERS] pgsql: Add trigonometric functions that work in degrees. |
Date: | 2016-04-26 03:25:54 |
Message-ID: | 7978.1461641154@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
I wrote:
> Evidently, asin() is returning an 80-bit result, and that's not
> getting rounded to double width before we divide by asin_0_5.
I found that I could reproduce this type of assembly-code behavior
on dromedary (gcc version 4.2.1) by using -mfpmath=387. That box
doesn't show the visible regression-test failure, but that must be
down to its version of asin() not producing the same low-order bits
that yours does. It's clear from the assembly output that the code
*would* misbehave given an appropriate asin() library function.
With that version of gcc, just casting the function output to double
changes nothing. The local-variable solution likewise. I do get
a useful fix if I declare the asin_x local variable volatile:
.loc 1 1873 0
fstpl (%esp)
call _asin
+ fstpl -16(%ebp)
+ LVL107:
+ fldl -16(%ebp)
fdivl _asin_0_5-"L00000000019$pb"(%ebx)
fmuls LC21-"L00000000019$pb"(%ebx)
Interestingly, declaring asin_x as global is not enough to fix it,
because it stores into the global but doesn't fetch back:
.loc 1 1873 0
fstpl (%esp)
call _asin
+ movl L_asin_x$non_lazy_ptr-"L00000000019$pb"(%ebx), %eax
+ fstl (%eax)
fdivl _asin_0_5-"L00000000019$pb"(%ebx)
fmuls LC21-"L00000000019$pb"(%ebx)
Declaring asin_x as a volatile global does fix it:
.loc 1 1873 0
fstpl (%esp)
call _asin
+ movl L_asin_x$non_lazy_ptr-"L00000000019$pb"(%ebx), %eax
+ fstpl (%eax)
+ fldl (%eax)
fdivl _asin_0_5-"L00000000019$pb"(%ebx)
fmuls LC21-"L00000000019$pb"(%ebx)
but that seems like just being uglier without much redeeming value.
In short, these tests suggest that we need a coding pattern like
this:
volatile float8 asin_x = asin(x);
return (asin_x / asin_0_5) * 30.0;
We could drop the "volatile" by adopting -ffloat-store, but that
doesn't seem like a better answer, because -ffloat-store would
pessimize a lot of code elsewhere. (It causes a whole lot of
changes in float.c, for sure.)
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2016-04-26 08:39:12 | pgsql: Fix typo in comment |
Previous Message | Tom Lane | 2016-04-25 23:13:16 | Re: [COMMITTERS] pgsql: Add trigonometric functions that work in degrees. |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2016-04-26 03:39:37 | Re: Suspicious behaviour on applying XLOG_HEAP2_VISIBLE. |
Previous Message | Andres Freund | 2016-04-26 02:47:19 | Re: Suspicious behaviour on applying XLOG_HEAP2_VISIBLE. |