Created attachment 181622 [details] patch Both IEEE-754 2008 and ISO/IEC TS 18661-4 define the half-cycle trignometric functions cospi, sinpi, and tanpi. The attached patch implements cospi[fl], sinpi[fl], and tanpi[fl]. Limited testing on the cospi and sinpi reveal a max ULP less than 0.89; while tanpi is more problematic with a max ULP less than 2.01 in the interval [0,0.5]. The algorithms used in these functions are documented in {ks}_cospi.c, {ks}_sinpi.c, and s_tanpi.c. Note 1. ISO/IEC TS 18661-4 says these funstions are guarded by a predefine macro. I have no idea or interest in what clang and gcc do with regards to this macro. I've put the functions behind __BSD_VISIBLE. Note 2. I no longer have access to a system with ld128 and adequate support to compile and test the ld128 implementations of these functions. Given the almost complete lack of input from others on improvements to libm, I doubt that anyone cares. If someone does care, the ld128 files contain a number of FIXME comments, and in particular, while the polynomial coefficients are given I did not update the polynomial algorithms to properly use the coefficients.
Created attachment 182139 [details] New patch with updated algorithms The attach patch supersedes the previously submitted patch.
Created attachment 182648 [details] new diff New patch
Got an email noting that Ed added himself to the CC list. Just a quick note. I have had time to work on this recently. Bruce Evans and I have a fairly long discussion in freebsd-numerics list concerning improvements.
I am happy to shepherd this into the tree when ready.
No reason to cause self-inflicted concussions.
Created attachment 229008 [details] New updated patch New diff. This patch is an updated version of the previously submitted code. A version of this code has been in my tree for a long time. In addition, the MPFR developers used this code as part of testing their recent implementations of the cosu(), sinu(), and tanu() functions.
Re-open PR to hopefully see the code finally committed. The initial comment still applies.
You should use FBSD_1.7 for 14.0 symbols namespace.
(In reply to Konstantin Belousov from comment #8) > You should use FBSD_1.7 for 14.0 symbols namespace. Okay. I won't bother asking "why?" To whomever actually steps up to commit the patch, please edit the diff before applying to change 1.6 to 1.7. If you apply the diff first, then please edit Symbol.map to change 1.6 and 1.7.
(In reply to Steve Kargl from comment #9) You put the new definitions under C99 clause, which seems to be wrong. Even C17 does not define the functions. In other words, they should be under _BSD_VISIBLE block.
Created attachment 229010 [details] new new updated patch Fix.
(In reply to Steve Kargl from comment #11) I will commit the patch after tinderbox finishes.
This seems to break on arches where long double is double AKA 64bit. For instance, on arm I see ===> lib/msun (obj,all,install) make[6]: make[6]: don't know how to make s_cospil.c. Stop make[6]: make[6]: don't know how to make s_sinpil.c. Stop make[6]: make[6]: don't know how to make s_tanpil.c. Stop
While on ld128: /usr/home/konstantinb/build/bsd/DEV/src/lib/msun/ld128/s_sinpil.c:63:19: error: expected ';' after expression hi *= 0x1p113L ^ ; 1 error generated. --- s_sinpil.o --- *** [s_sinpil.o] Error code 1
(In reply to Konstantin Belousov from comment #13) Whoops, sorry about that. Juggling 4 year diffs and re-integrating them into a git world problem. In the diff I had +# IEEE-754 2008 and ISO/IEC TS 18661-4 half-cycle trignometric functions +COMMON_SRCS+= s_cospi.c s_cospif.c s_cospil.c \ + s_sinpi.c s_sinpif.c s_sinpil.c \ + s_tanpi.c s_tanpif.c s_tanpil.c The long double function should have been added under the .if ${LDBL_PREC} != 53 # If long double != double use these; otherwise, we alias the double versions. part of the Makefile. Do you want me to regenerate a diff.
(In reply to Konstantin Belousov from comment #14) L> While on ld128: > /usr/home/konstantinb/build/bsd/DEV/src/lib/msun/ld128/s_sinpil.c:63:19: error: > expected ';' after expression > hi *= 0x1p113L See the initial post about the ld128 version. The ld128 code is untested and hasn't even been compiled by me as I no longer have access to a ld128 system. flame.freebsd.org was decommissioned years ago. Is there a ld128 system available from freefall.freebsd.org?
(In reply to Steve Kargl from comment #16) I think there are powerpc64 systems there, but I am not aware of the hosts names. I made the changes locally, added missed ';' and rearranged Makefile. I will post a new patch there after tinderbox is happy, or there is something that is not clear to me how to fix.
I was told ref14-powerpc64.freebsd.org is there.
Created attachment 229037 [details] Version of the patch that passed tinderbox This is the updated version of the patch, with only changes against the original version are: - syntax error in ld128 fixed (mised ';') - addition of .c files moved under .if ${LDBL_PREC} != 53 Please state if you are fine with committing this one
Kib, I have done exhaustive testing on the float versions and tested 100s of millions, if not billions, of values for double and ld80. I just checked and I can log into ref14-power64. If you prefer that I do some testing on ref14-power64 for the ld128 code prior to committing the patch, I can do that over the next few days. Otherwise, I'm comfortable with committing the tinderbox-tested code. I'll do post-commit testing of ld128 and submit follow-up patches if need be. -- steve
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=dce5f3abed7181cc533ca5ed3de44517775e78dd commit dce5f3abed7181cc533ca5ed3de44517775e78dd Author: Steve Kargl <kargl@FreeBSD.org> AuthorDate: 2021-10-25 13:13:52 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-10-25 23:50:20 +0000 [LIBM] implementations of sinpi[fl], cospi[fl], and tanpi[fl] Both IEEE-754 2008 and ISO/IEC TS 18661-4 define the half-cycle trignometric functions cospi, sinpi, and tanpi. The attached patch implements cospi[fl], sinpi[fl], and tanpi[fl]. Limited testing on the cospi and sinpi reveal a max ULP less than 0.89; while tanpi is more problematic with a max ULP less than 2.01 in the interval [0,0.5]. The algorithms used in these functions are documented in {ks}_cospi.c, {ks}_sinpi.c, and s_tanpi.c. Note. I no longer have access to a system with ld128 and adequate support to compile and test the ld128 implementations of these functions. Given the almost complete lack of input from others on improvements to libm, I doubt that anyone cares. If someone does care, the ld128 files contain a number of FIXME comments, and in particular, while the polynomial coefficients are given I did not update the polynomial algorithms to properly use the coefficients. PR: 218514 MFC after: 2 weeks lib/msun/Makefile | 14 +++- lib/msun/Symbol.map | 13 +++ lib/msun/ld128/k_cospil.h (new) | 42 ++++++++++ lib/msun/ld128/k_sinpil.h (new) | 42 ++++++++++ lib/msun/ld128/s_cospil.c (new) | 109 +++++++++++++++++++++++++ lib/msun/ld128/s_sinpil.c (new) | 118 +++++++++++++++++++++++++++ lib/msun/ld128/s_tanpil.c (new) | 120 +++++++++++++++++++++++++++ lib/msun/ld80/k_cospil.h (new) | 42 ++++++++++ lib/msun/ld80/k_sinpil.h (new) | 42 ++++++++++ lib/msun/ld80/s_cospil.c (new) | 129 +++++++++++++++++++++++++++++ lib/msun/ld80/s_sinpil.c (new) | 140 ++++++++++++++++++++++++++++++++ lib/msun/ld80/s_tanpil.c (new) | 139 +++++++++++++++++++++++++++++++ lib/msun/man/cospi.3 (new) | 111 +++++++++++++++++++++++++ lib/msun/man/sinpi.3 (new) | 102 +++++++++++++++++++++++ lib/msun/man/tanpi.3 (new) | 106 ++++++++++++++++++++++++ lib/msun/src/k_cospi.h (new) | 44 ++++++++++ lib/msun/src/k_sinpi.h (new) | 43 ++++++++++ lib/msun/src/math.h | 9 ++ lib/msun/src/s_cospi.c (new) | 151 ++++++++++++++++++++++++++++++++++ lib/msun/src/s_cospif.c (new) | 109 +++++++++++++++++++++++++ lib/msun/src/s_sinpi.c (new) | 168 ++++++++++++++++++++++++++++++++++++++ lib/msun/src/s_sinpif.c (new) | 122 ++++++++++++++++++++++++++++ lib/msun/src/s_tanpi.c (new) | 176 ++++++++++++++++++++++++++++++++++++++++ lib/msun/src/s_tanpif.c (new) | 114 ++++++++++++++++++++++++++ 24 files changed, 2203 insertions(+), 2 deletions(-)
(In reply to Steve Kargl from comment #20) I do not think it would be productive to let this patch sit more in bugzilla. Fix ld128 as needed, and provide with the any updates when ready.
Created attachment 229047 [details] Put files in right Makefile locations Kib, I removed my old WIP and pulled /usr/src this morning. Upon building a libm, I noticed that we were only building the files if the compiler was gcc. The nested .if ... .endif seems to have caused us problems. The attached diff moves the files to appropriate locations in the Makefile. On AMD64, after building and installing new libs, I see % nm /usr/lib/libm.a | grep pi | grep s_ s_cospi.o: s_cospif.o: s_sinpi.o: s_sinpif.o: s_tanpi.o: s_tanpif.o: s_cospil.o: s_sinpil.o: s_tanpil.o:
For those curious, with older testing programs I'm see Max ULP as shown in the tables below. sinpif time: 0.0077 usec per call (average [0x1p-14,0.25] exhaustive) cospif time: 0.0063 usec per call tanpif time: 0.0078 usec per call Interval sinpif cospif tanpif [0x1p-14,0.25] 0.500074 0.500898 0.799714 (exhaustive) [0.25,1] 0.500898 0.500893 0.799635 (exhaustive) [100,104] 0.500893 0.500893 0.796762 (exhaustive) sinpi time: 0.0159 usec per call (1M calls in [0x1p-29,0.25]) cospi time: 0.0145 usec per call tanpi time: 0.0264 usec per call Interval sinpi cospi tanpi [0x1p-29,0.25] 0.735554 0.791773 0.863064 (total tested: 8388607) [0.25,1] 0.767862 0.767862 0.868273 (total tested: 8388606) [100,104] 0.769793 0.765948 0.855805 (total tested: 8388604) ld80/sinpil time: 0.1046 usec per call (1M calls in [0x1p-34,0.25]) ld80/cospil time: 0.1014 usec per call ld80/tanpil time: 0.1915 usec per call Interval sinpil cospil tanpil [0x1p-34,0.25] 0.710709 0.773680 0.870418 (total tested: 8388607) [0.25,1] 0.769538 0.765667 0.863644 (total tested: 8388606) [100,104] 0.765416 0.763284 0.861817 (total tested: 8388604)
Created attachment 229051 [details] More build fixes: Makefile (by kargl) and msun/ld128/s_tanpil.c The attached patch is what I am going to commit. Please confirm.
(In reply to Konstantin Belousov from comment #25) Kib, That looks correct. I should be able to start testing ld128 later in the week. Apologies for the issues. Merging files from my old svn repository in the git repository clearly was munged.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=ca3d8cb087cd5b40369478b1693f3e4038b5fa23 commit ca3d8cb087cd5b40369478b1693f3e4038b5fa23 Author: Steve Kargl <kargl@FreeBSD.org> AuthorDate: 2021-10-26 20:53:51 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-10-26 22:34:12 +0000 lib/msun: Move the files to appropriate locations in the Makefile Fixes: dce5f3abed7181cc533ca5ed PR: 218514 MFC after: 1 week lib/msun/Makefile | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
Created attachment 229108 [details] float.h needed for week reference The attached patch fixes the omission of '#include <float.h>', which is needed for the weak reference on systems with LDBL_MANT_DIG == DBL_MANT_DIG. This was found by compiling on ref14-powerpc.freebsd.org. Still in search of an ld128 system.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=3bfc837685b8128067b946b31dfe2120dae0d003 commit 3bfc837685b8128067b946b31dfe2120dae0d003 Author: Steve Kargl <kargl@FreeBSD.org> AuthorDate: 2021-10-28 22:53:13 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-10-29 00:15:19 +0000 sinpi,cospi,tanpi: float.h needed for week reference The patch fixes the omission of '#include <float.h>', which is needed for the weak reference on systems with LDBL_MANT_DIG == DBL_MANT_DIG. PR: 218514 MFC after: 1 week lib/msun/src/s_cospi.c | 1 + lib/msun/src/s_sinpi.c | 1 + lib/msun/src/s_tanpi.c | 1 + 3 files changed, 3 insertions(+)
Created attachment 229180 [details] Fix the ld128 implementations Mark Murray graciously provided access to an aarch64 system to test the ld128 implementations. This patch address * Misuses of copysignl() in sinpil() and tanpil(). * Redo the splitting of argument 'x' into an integer part and remainder. The remainder must satify 0 <= r < 1. * Update the reduction of the integer part to something that can easily be seen as even or odd, e.g., sin(pi*x) = (-1)^n*sin(pi*r) with n <= 2^112 and we an reduce n by subtracting integer powers of 2. * In s_cospil.c, fix typos where 'x' is used where 'ax', the remainder, is required. * In tanpil(), fix the use of an uninitialized variable, ax = fabsl(ax), ax should be x in fabsl(). One item of note, in the limited tested on aarch64, the max ULP for sinpil() and cospil() were less than 1.1 ULP, which is higher that the desired max ULP less than 1. This was traced to the kernel for cosl() in the fundamental interval [0,pi/4]. The coefficients in the minmax polynomial likely need refinement.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=4f889260c33c163ab28e0e082b4d7e7562d9c647 commit 4f889260c33c163ab28e0e082b4d7e7562d9c647 Author: Steve Kargl <kargl@FreeBSD.org> AuthorDate: 2021-10-31 22:26:20 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-11-01 02:38:19 +0000 sinpi[fl] etc: Fix the ld128 implementations Mark Murray graciously provided access to an aarch64 system to test the ld128 implementations. This patch address * Misuses of copysignl() in sinpil() and tanpil(). * Redo the splitting of argument 'x' into an integer part and remainder. The remainder must satify 0 <= r < 1. * Update the reduction of the integer part to something that can easily be seen as even or odd, e.g., sin(pi*x) = (-1)^n*sin(pi*r) with n <= 2^112 and we an reduce n by subtracting integer powers of 2. * In s_cospil.c, fix typos where 'x' is used where 'ax', the remainder, is required. * In tanpil(), fix the use of an uninitialized variable, ax = fabsl(ax), ax should be x in fabsl(). One item of note, in the limited tested on aarch64, the max ULP for sinpil() and cospil() were less than 1.1 ULP, which is higher that the desired max ULP less than 1. This was traced to the kernel for cosl() in the fundamental interval [0,pi/4]. The coefficients in the minmax polynomial likely need refinement. PR: 218514 MFC after: 1 week lib/msun/ld128/s_cospil.c | 31 +++++++++++++++++-------------- lib/msun/ld128/s_sinpil.c | 29 ++++++++++++++++------------- lib/msun/ld128/s_tanpil.c | 23 +++++++++++++---------- 3 files changed, 46 insertions(+), 37 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=a49617abb0c0dc763712fd70fae8bbd66ecca835 commit a49617abb0c0dc763712fd70fae8bbd66ecca835 Author: Steve Kargl <kargl@FreeBSD.org> AuthorDate: 2021-10-25 13:13:52 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-11-02 01:16:29 +0000 [LIBM] implementations of sinpi[fl], cospi[fl], and tanpi[fl] PR: 218514 (cherry picked from commit dce5f3abed7181cc533ca5ed3de44517775e78dd) lib/msun/Makefile | 14 +++- lib/msun/Symbol.map | 13 +++ lib/msun/ld128/k_cospil.h (new) | 42 ++++++++++ lib/msun/ld128/k_sinpil.h (new) | 42 ++++++++++ lib/msun/ld128/s_cospil.c (new) | 109 +++++++++++++++++++++++++ lib/msun/ld128/s_sinpil.c (new) | 118 +++++++++++++++++++++++++++ lib/msun/ld128/s_tanpil.c (new) | 120 +++++++++++++++++++++++++++ lib/msun/ld80/k_cospil.h (new) | 42 ++++++++++ lib/msun/ld80/k_sinpil.h (new) | 42 ++++++++++ lib/msun/ld80/s_cospil.c (new) | 129 +++++++++++++++++++++++++++++ lib/msun/ld80/s_sinpil.c (new) | 140 ++++++++++++++++++++++++++++++++ lib/msun/ld80/s_tanpil.c (new) | 139 +++++++++++++++++++++++++++++++ lib/msun/man/cospi.3 (new) | 111 +++++++++++++++++++++++++ lib/msun/man/sinpi.3 (new) | 102 +++++++++++++++++++++++ lib/msun/man/tanpi.3 (new) | 106 ++++++++++++++++++++++++ lib/msun/src/k_cospi.h (new) | 44 ++++++++++ lib/msun/src/k_sinpi.h (new) | 43 ++++++++++ lib/msun/src/math.h | 9 ++ lib/msun/src/s_cospi.c (new) | 151 ++++++++++++++++++++++++++++++++++ lib/msun/src/s_cospif.c (new) | 109 +++++++++++++++++++++++++ lib/msun/src/s_sinpi.c (new) | 168 ++++++++++++++++++++++++++++++++++++++ lib/msun/src/s_sinpif.c (new) | 122 ++++++++++++++++++++++++++++ lib/msun/src/s_tanpi.c (new) | 176 ++++++++++++++++++++++++++++++++++++++++ lib/msun/src/s_tanpif.c (new) | 114 ++++++++++++++++++++++++++ 24 files changed, 2203 insertions(+), 2 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=5033d0b56f5be90fcca4ede92f28f9335eb20b4a commit 5033d0b56f5be90fcca4ede92f28f9335eb20b4a Author: Steve Kargl <kargl@FreeBSD.org> AuthorDate: 2021-10-31 22:26:20 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-11-02 01:16:29 +0000 sinpi[fl] etc: Fix the ld128 implementations PR: 218514 (cherry picked from commit 4f889260c33c163ab28e0e082b4d7e7562d9c647) lib/msun/ld128/s_cospil.c | 31 +++++++++++++++++-------------- lib/msun/ld128/s_sinpil.c | 29 ++++++++++++++++------------- lib/msun/ld128/s_tanpil.c | 23 +++++++++++++---------- 3 files changed, 46 insertions(+), 37 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=8b231404a1b29c1a21636d4db1ee1e454a87e54c commit 8b231404a1b29c1a21636d4db1ee1e454a87e54c Author: Steve Kargl <kargl@FreeBSD.org> AuthorDate: 2021-10-28 22:53:13 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-11-02 01:16:29 +0000 sinpi,cospi,tanpi: float.h needed for week reference PR: 218514 (cherry picked from commit 3bfc837685b8128067b946b31dfe2120dae0d003) lib/msun/src/s_cospi.c | 1 + lib/msun/src/s_sinpi.c | 1 + lib/msun/src/s_tanpi.c | 1 + 3 files changed, 3 insertions(+)
Created attachment 229215 [details] Final diff and this PR can be closed As mention previously, the minmax polynomial approximation in the kernel for cosl() seem to have a bad set of coefficients. In testing, cosl() in the interval [0.785, pi/4] for 1 million values and pi/4 written to 37 decimal digits. The old version on an aarch64 system gave % tlibm/tlibm_lmath -l -x 0.78 -X 7.85398163397448309615660845819875721e-1L cos Interval tested for cosl: [0.78,0.785398] count: 1000000 xm = 7.80213913234863919029058821396125599e-01L libm = 7.10763080972549562455058499280609083e-01L mpfr = 7.10763080972549562455058499280608983e-01L ULP = 1.04431 The max ULP exceeds 1, which is not good. So, I rinsed off a 10 year code and recomputed coefficients. The new minmax polynomial now yields tlibm/tlibm_lmath -l -x 0.78 -X 7.85398163397448309615660845819875721e-1L cos Interval tested for cosl: [0.78,0.785398] count: 1000000 xm = 7.82916198746768272588844890973704219e-01L libm = 7.08859615479571058183956453286628396e-01L mpfr = 7.08859615479571058183956453286628469e-01L ULP = 0.75407 which is very good. The attach fixes the coefficients.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6d04e1422e70ca0a77552782c01c291f90716773 commit 6d04e1422e70ca0a77552782c01c291f90716773 Author: Steve Kargl <kargl@FreeBSD.org> AuthorDate: 2021-11-02 08:54:10 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-11-02 08:54:10 +0000 cosl(): fix polynomial approximation coefficients for ld128 version As mention previously, the minmax polynomial approximation in the kernel for cosl() seem to have a bad set of coefficients. In testing, cosl() in the interval [0.785, pi/4] for 1 million values and pi/4 written to 37 decimal digits. The old version on an aarch64 system gave % tlibm/tlibm_lmath -l -x 0.78 -X 7.85398163397448309615660845819875721e-1L cos Interval tested for cosl: [0.78,0.785398] count: 1000000 xm = 7.80213913234863919029058821396125599e-01L libm = 7.10763080972549562455058499280609083e-01L mpfr = 7.10763080972549562455058499280608983e-01L ULP = 1.04431 The max ULP exceeds 1, which is not good. So, I rinsed off a 10 year code and recomputed coefficients. The new minmax polynomial now yields % tlibm/tlibm_lmath -l -x 0.78 -X 7.85398163397448309615660845819875721e-1L cos Interval tested for cosl: [0.78,0.785398] count: 1000000 xm = 7.82916198746768272588844890973704219e-01L libm = 7.08859615479571058183956453286628396e-01L mpfr = 7.08859615479571058183956453286628469e-01L ULP = 0.75407 which is very good. PR: 218514 MFC after: 1 week lib/msun/ld128/k_cosl.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=ced974a7b07de731d8cce8d15be9968ece74ff3d commit ced974a7b07de731d8cce8d15be9968ece74ff3d Author: Steve Kargl <kargl@FreeBSD.org> AuthorDate: 2021-11-02 08:54:10 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-11-08 01:49:45 +0000 cosl(): fix polynomial approximation coefficients for ld128 version PR: 218514 (cherry picked from commit 6d04e1422e70ca0a77552782c01c291f90716773) lib/msun/ld128/k_cosl.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-)