Re: [PATCH] Recent libm additions
This is now PR 229876. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=229876 -- steve On Mon, Jul 16, 2018 at 08:20:11PM -0700, Steve Kargl wrote: > Version 2. After applying the patch, one can > > % svn delete libm/msun/src/polevll.c > % svn commit libm/msun/src/polevll.c > > * lib/msun/Makefile: > . Remove polevll.c > > * lib/msun/ld80/e_powl.c: > . Copy contents of polevll.c to here. This is the only consumer of > these functions. Make functions 'static inline'. > . Make reducl a 'static inline' function. > > * lib/msun/man/exp.3: > . Remove BUGS section that no longer applies. > > * lib/msun/src/math_private.h: > . Remove prototypes of __p1evll() and __polevll() > > * lib/msun/src/s_cpow.c: > * lib/msun/src/s_cpowf.c: > * lib/msun/src/s_cpowl.c > . Use the CMPLX macro from either C99 or math_private.h (depends of > compiler support) instead of the problematic use of complex I. > > Index: lib/msun/Makefile > === > --- lib/msun/Makefile (revision 336360) > +++ lib/msun/Makefile (working copy) > @@ -17,6 +17,8 @@ > > .include "${ARCH_SUBDIR}/Makefile.inc" > > +CFLAGS+=-msse > + > .PATH: ${.CURDIR}/${ARCH_SUBDIR} > .if ${MACHINE_CPUARCH} == "i386" || ${MACHINE_CPUARCH} == "amd64" > .PATH: ${.CURDIR}/x86 > @@ -56,7 +58,6 @@ > imprecise.c \ > k_cos.c k_cosf.c k_exp.c k_expf.c k_rem_pio2.c k_sin.c k_sinf.c \ > k_tan.c k_tanf.c \ > - polevll.c \ > s_asinh.c s_asinhf.c s_atan.c s_atanf.c s_carg.c s_cargf.c s_cargl.c \ > s_cbrt.c s_cbrtf.c s_ceil.c s_ceilf.c s_clog.c s_clogf.c \ > s_copysign.c s_copysignf.c s_cos.c s_cosf.c \ > Index: lib/msun/ld80/e_powl.c > === > --- lib/msun/ld80/e_powl.c(revision 336360) > +++ lib/msun/ld80/e_powl.c(working copy) > @@ -14,6 +14,52 @@ > * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > */ > > +#include > +__FBSDID("$FreeBSD$"); > + > +#include > + > +#include "math_private.h" > + > +/* > + * Polynomial evaluator: > + * P[0] x^n + P[1] x^(n-1) + ... + P[n] > + */ > +static inline long double > +__polevll(long double x, long double *PP, int n) > +{ > + long double y; > + long double *P; > + > + P = PP; > + y = *P++; > + do { > + y = y * x + *P++; > + } while (--n); > + > + return (y); > +} > + > +/* > + * Polynomial evaluator: > + * x^n + P[0] x^(n-1) + P[1] x^(n-2) + ... + P[n] > + */ > +static inline long double > +__p1evll(long double x, long double *PP, int n) > +{ > + long double y; > + long double *P; > + > + P = PP; > + n -= 1; > + y = x + *P++; > + do { > + y = y * x + *P++; > + } while (--n); > + > + return (y); > +} > + > /* powl.c > * > * Power function, long double precision > @@ -467,7 +513,7 @@ > > > /* Find a multiple of 1/NXT that is within 1/NXT of x. */ > -static long double > +static inline long double > reducl(long double x) > { > long double t; > Index: lib/msun/man/exp.3 > === > --- lib/msun/man/exp.3(revision 336360) > +++ lib/msun/man/exp.3(working copy) > @@ -180,16 +180,9 @@ > then \*(Na**0 = 1 too because x**0 = 1 for all finite > and infinite x, i.e., independently of x. > .El > -.Sh BUGS > -To conform with newer C/C++ standards, a stub implementation for > -.Nm powl > -was committed to the math library, where > -.Nm powl > -is mapped to > -.Nm pow . > -Thus, the numerical accuracy is at most that of the 53-bit double > -precision implementation. > .Sh SEE ALSO > +.Xr clog 3 > +.Xr cpow 3 > .Xr fenv 3 , > .Xr ldexp 3 , > .Xr log 3 , > Index: lib/msun/src/math_private.h > === > --- lib/msun/src/math_private.h (revision 336360) > +++ lib/msun/src/math_private.h (working copy) > @@ -828,7 +828,4 @@ > long double __kernel_cosl(long double, long double); > long double __kernel_tanl(long double, long double, int); > > -long double __p1evll(long double, void *, int); > -long double __polevll(long double, void *, int); > - > #endif /* !_MATH_PRIVATE_H_ */ > Index: lib/msun/src/s_cpow.c > === > --- lib/msun/src/s_cpow.c (revision 336360) > +++ lib/msun/src/s_cpow.c (working copy) > @@ -60,7 +60,7 @@ > y = cimag (z); > absa = cabs (a); > if (absa == 0.0) { > - return (0.0 + 0.0 * I); > + return (CMPLX(0.0, 0.0)); > } > arga = carg (a); > r = pow (absa, x); > @@ -69,6 +69,6 @@ > r = r * exp (-y * arga); > theta = theta + y * log (absa); > } > - w = r * cos (theta) + (r * sin (theta)) * I; > + w = CMPLX(r * c
Re: [PATCH] Recent libm additions
Version 2. After applying the patch, one can % svn delete libm/msun/src/polevll.c % svn commit libm/msun/src/polevll.c * lib/msun/Makefile: . Remove polevll.c * lib/msun/ld80/e_powl.c: . Copy contents of polevll.c to here. This is the only consumer of these functions. Make functions 'static inline'. . Make reducl a 'static inline' function. * lib/msun/man/exp.3: . Remove BUGS section that no longer applies. * lib/msun/src/math_private.h: . Remove prototypes of __p1evll() and __polevll() * lib/msun/src/s_cpow.c: * lib/msun/src/s_cpowf.c: * lib/msun/src/s_cpowl.c . Use the CMPLX macro from either C99 or math_private.h (depends of compiler support) instead of the problematic use of complex I. Index: lib/msun/Makefile === --- lib/msun/Makefile (revision 336360) +++ lib/msun/Makefile (working copy) @@ -17,6 +17,8 @@ .include "${ARCH_SUBDIR}/Makefile.inc" +CFLAGS+=-msse + .PATH: ${.CURDIR}/${ARCH_SUBDIR} .if ${MACHINE_CPUARCH} == "i386" || ${MACHINE_CPUARCH} == "amd64" .PATH: ${.CURDIR}/x86 @@ -56,7 +58,6 @@ imprecise.c \ k_cos.c k_cosf.c k_exp.c k_expf.c k_rem_pio2.c k_sin.c k_sinf.c \ k_tan.c k_tanf.c \ - polevll.c \ s_asinh.c s_asinhf.c s_atan.c s_atanf.c s_carg.c s_cargf.c s_cargl.c \ s_cbrt.c s_cbrtf.c s_ceil.c s_ceilf.c s_clog.c s_clogf.c \ s_copysign.c s_copysignf.c s_cos.c s_cosf.c \ Index: lib/msun/ld80/e_powl.c === --- lib/msun/ld80/e_powl.c (revision 336360) +++ lib/msun/ld80/e_powl.c (working copy) @@ -14,6 +14,52 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#include +__FBSDID("$FreeBSD$"); + +#include + +#include "math_private.h" + +/* + * Polynomial evaluator: + * P[0] x^n + P[1] x^(n-1) + ... + P[n] + */ +static inline long double +__polevll(long double x, long double *PP, int n) +{ + long double y; + long double *P; + + P = PP; + y = *P++; + do { + y = y * x + *P++; + } while (--n); + + return (y); +} + +/* + * Polynomial evaluator: + * x^n + P[0] x^(n-1) + P[1] x^(n-2) + ... + P[n] + */ +static inline long double +__p1evll(long double x, long double *PP, int n) +{ + long double y; + long double *P; + + P = PP; + n -= 1; + y = x + *P++; + do { + y = y * x + *P++; + } while (--n); + + return (y); +} + /* powl.c * * Power function, long double precision @@ -467,7 +513,7 @@ /* Find a multiple of 1/NXT that is within 1/NXT of x. */ -static long double +static inline long double reducl(long double x) { long double t; Index: lib/msun/man/exp.3 === --- lib/msun/man/exp.3 (revision 336360) +++ lib/msun/man/exp.3 (working copy) @@ -180,16 +180,9 @@ then \*(Na**0 = 1 too because x**0 = 1 for all finite and infinite x, i.e., independently of x. .El -.Sh BUGS -To conform with newer C/C++ standards, a stub implementation for -.Nm powl -was committed to the math library, where -.Nm powl -is mapped to -.Nm pow . -Thus, the numerical accuracy is at most that of the 53-bit double -precision implementation. .Sh SEE ALSO +.Xr clog 3 +.Xr cpow 3 .Xr fenv 3 , .Xr ldexp 3 , .Xr log 3 , Index: lib/msun/src/math_private.h === --- lib/msun/src/math_private.h (revision 336360) +++ lib/msun/src/math_private.h (working copy) @@ -828,7 +828,4 @@ long double __kernel_cosl(long double, long double); long double __kernel_tanl(long double, long double, int); -long double __p1evll(long double, void *, int); -long double __polevll(long double, void *, int); - #endif /* !_MATH_PRIVATE_H_ */ Index: lib/msun/src/s_cpow.c === --- lib/msun/src/s_cpow.c (revision 336360) +++ lib/msun/src/s_cpow.c (working copy) @@ -60,7 +60,7 @@ y = cimag (z); absa = cabs (a); if (absa == 0.0) { - return (0.0 + 0.0 * I); + return (CMPLX(0.0, 0.0)); } arga = carg (a); r = pow (absa, x); @@ -69,6 +69,6 @@ r = r * exp (-y * arga); theta = theta + y * log (absa); } - w = r * cos (theta) + (r * sin (theta)) * I; + w = CMPLX(r * cos (theta), r * sin (theta)); return (w); } Index: lib/msun/src/s_cpowf.c === --- lib/msun/src/s_cpowf.c (revision 336360) +++ lib/msun/src/s_cpowf.c (working copy) @@ -59,7 +59,7 @@ y = cimagf(z); absa = cabsf (a); if (absa == 0.0f) { - return (0.0f + 0.0f * I); + return (CMPLXF(0.0f, 0.0f));
Re: [PATCH] Recent libm additions
Julia's libm is https://github.com/JuliaMath/openlibm (or http://openlibm.org/) The do try merging latest from freebsd (msun), although it seems not the very latest. (see e.g. https://github.com/JuliaMath/openlibm/pull/118) I did try installing the master git branch of openlibm on FreeBSD 11.1 with clang, it builds and passes all tests they have. Does it make sense to contribute there more tests (if any --- they seem to have quite a list), to clear suspicions about precision? On Sun, Jul 15, 2018 at 10:03 PM Matthew Macy wrote: > > On Sun, Jul 15, 2018 at 12:43 PM, Montgomery-Smith, Stephen > wrote: > > On 07/15/2018 02:09 PM, Warner Losh wrote: > >> I'm not saying that he has a lock. I'm saying he's are domain expert and > >> many mistakes can be avoided by talking to him. > >> > >> I'm saying we have history here, and that history, while poorly documented, > >> wasn't followed. To the extent it is poorly documented, we should fix that. > >> > >> Warner > >> > > I agree that we should document the process. Maybe also include > > freebsd-numerics@ on these discussions, as that is why it was created. > > > > But I'm really glad these changes were committed. I have found the > > people tend to drag their feet a lot on numerics issues. > > > > Has anyone done an analysis of the OpenBSD powl functions from an > > accuracy point of view? That is, to test how many ULP of error these > > functions have? If not, I could give it a go, although not for several > > months because life is very busy. > > They're also used by Julia. You might ask there first. > -M > ___ > freebsd-numer...@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-numerics > To unsubscribe, send any mail to "freebsd-numerics-unsubscr...@freebsd.org" ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 01:09:41PM -0600, Warner Losh wrote: > I'm not saying that he has a lock. I'm saying he's are domain expert > and many mistakes can be avoided by talking to him. fwiw, substantially all the work done since at least 2013 is from kargl. (I am eliding the licensing, Makefile, and typo commits.) https://svnweb.freebsd.org/base/head/lib/msun/src/?sortby=date#dirlist mcl ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 12:43 PM, Montgomery-Smith, Stephen wrote: > On 07/15/2018 02:09 PM, Warner Losh wrote: >> I'm not saying that he has a lock. I'm saying he's are domain expert and >> many mistakes can be avoided by talking to him. >> >> I'm saying we have history here, and that history, while poorly documented, >> wasn't followed. To the extent it is poorly documented, we should fix that. >> >> Warner >> > I agree that we should document the process. Maybe also include > freebsd-numerics@ on these discussions, as that is why it was created. > > But I'm really glad these changes were committed. I have found the > people tend to drag their feet a lot on numerics issues. > > Has anyone done an analysis of the OpenBSD powl functions from an > accuracy point of view? That is, to test how many ULP of error these > functions have? If not, I could give it a go, although not for several > months because life is very busy. They're also used by Julia. You might ask there first. -M ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 02:00:37PM -0700, Matthew Macy wrote: > On Sun, Jul 15, 2018 at 12:43 PM, Montgomery-Smith, Stephen > wrote: > > On 07/15/2018 02:09 PM, Warner Losh wrote: > >> I'm not saying that he has a lock. I'm saying he's are domain expert and > >> many mistakes can be avoided by talking to him. > >> > >> I'm saying we have history here, and that history, while poorly documented, > >> wasn't followed. To the extent it is poorly documented, we should fix that. > >> > >> Warner > >> > > I agree that we should document the process. Maybe also include > > freebsd-numerics@ on these discussions, as that is why it was created. > > > > But I'm really glad these changes were committed. I have found the > > people tend to drag their feet a lot on numerics issues. > > > > Has anyone done an analysis of the OpenBSD powl functions from an > > accuracy point of view? That is, to test how many ULP of error these > > functions have? If not, I could give it a go, although not for several > > months because life is very busy. > > They're also used by Julia. You might ask there first. You also need to fix the pow.3 documentation. It currently states BUGS To conform with newer C/C++ standards, a stub implementation for powl was committed to the math library, where powl is mapped to pow. Thus, the numerical accuracy is at most that of the 53-bit double precision implementation. -- Steve ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 02:00:37PM -0700, Matthew Macy wrote: > On Sun, Jul 15, 2018 at 12:43 PM, Montgomery-Smith, Stephen > wrote: > > On 07/15/2018 02:09 PM, Warner Losh wrote: > >> I'm not saying that he has a lock. I'm saying he's are domain expert and > >> many mistakes can be avoided by talking to him. > >> > >> I'm saying we have history here, and that history, while poorly documented, > >> wasn't followed. To the extent it is poorly documented, we should fix that. > >> > >> Warner > >> > > I agree that we should document the process. Maybe also include > > freebsd-numerics@ on these discussions, as that is why it was created. > > > > But I'm really glad these changes were committed. I have found the > > people tend to drag their feet a lot on numerics issues. > > > > Has anyone done an analysis of the OpenBSD powl functions from an > > accuracy point of view? That is, to test how many ULP of error these > > functions have? If not, I could give it a go, although not for several > > months because life is very busy. > > They're also used by Julia. You might ask there first. The FPU on i686-class hardware is set to use 53-bit precision. powl.c likely has at least a 2**11 ULP for a (large?) number of arguments. Go read the msun/src/math_private.h. You'll find LD80C for constucting long double literal constants, ENTERI() and RETURNI() marcos that toggle the precision of the FPU. These are used in ld80 code, e.g., e_lgammal_r.c. So, it doesn't matter what the Julia developers say unless their testing was done on FreeBSD. -- Steve ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 12:23:06PM -0700, Cy Schubert wrote: > I wasn't saying Steve has a lock however in case non-committers > might feel they do, addressing all points in my reply. Not saying > anyone feels this way today but we should consider this in whatever > we decide here (considering all possibilities). IMO adding subject > matter experts to MAINTAINERS seems like the easiest way to document > who might be the go-to person. I don't have a lock, and I don't want one. I do, however, make a part of my living using FreeBSD for floating point intensive research. I think that changes, including the addition of new functions, to libm should be reviewed and preferably tested. Grabbing code from OpenBSD (or anywhere), getting it to compile and integrated into the build infrastructure does not constitute a code review. There is a mailing list dedicated to numerics (aka floating point) on FreeBSD: freebsd-numer...@freebsd.org. -- Steve ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
RE: [PATCH] Recent libm additions
That'll work too. --- Sent using a tiny phone keyboard. Apologies for any typos and autocorrect. Also, this old phone only supports top post. Apologies. Cy Schubert or The need of the many outweighs the greed of the few. --- -Original Message- From: Warner Losh Sent: 15/07/2018 12:26 To: Cy Schubert Cc: Ian Lepore; K. Macy; Steve Kargl; FreeBSD Current Subject: Re: [PATCH] Recent libm additions So something like this: diff --git a/MAINTAINERS b/MAINTAINERS index 51d3688f8b8..3e6584f24a1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -58,6 +58,7 @@ kqueuejmg Pre-commit review requested. Documentation Required. libdpv dteske Pre-commit review requested. Keep in sync with dpv(1). libfetch des Pre-commit review requested, email only. libfigpar dteske Pre-commit review requested. +libm freebsd-numericsSend email with patches to freebsd-numerics@ libpam des Pre-commit review requested, email only. linprocfs des Pre-commit review requested, email only. lprgad Pre-commit review requested, particularly for is what you're suggesting? Warner On Sun, Jul 15, 2018 at 1:23 PM, Cy Schubert wrote: I wasn't saying Steve has a lock however in case non-committers might feel they do, addressing all points in my reply. Not saying anyone feels this way today but we should consider this in whatever we decide here (considering all possibilities). IMO adding subject matter experts to MAINTAINERS seems like the easiest way to document who might be the go-to person. --- Sent using a tiny phone keyboard. Apologies for any typos and autocorrect. Also, this old phone only supports top post. Apologies. Cy Schubert or The need of the many outweighs the greed of the few. --- From: Warner Losh Sent: 15/07/2018 12:09 To: Cy Schubert Cc: Ian Lepore; K. Macy; Steve Kargl; FreeBSD Current Subject: Re: [PATCH] Recent libm additions I'm not saying that he has a lock. I'm saying he's are domain expert and many mistakes can be avoided by talking to him. I'm saying we have history here, and that history, while poorly documented, wasn't followed. To the extent it is poorly documented, we should fix that. Warner On Sun, Jul 15, 2018 at 12:43 PM, Cy Schubert wrote: I don't think it makes sense for a non-committer to have a lock on anything in base. However a request for review makes a lot of sense. If a non-committer or former committer is the SME on a particular subject it's best that they be consulted even if they don't request it. IMO more input is better. Where better to document this than in MAINTAINERS. Having said all this. If a person is a former committer and it's not documented, how are we to know? If people agree, should we start documenting SMEs in MAINTAINERS? --- Sent using a tiny phone keyboard. Apologies for any typos and autocorrect. Also, this old phone only supports top post. Apologies. Cy Schubert or The need of the many outweighs the greed of the few. --- From: Ian Lepore Sent: 15/07/2018 11:08 To: Warner Losh; K. Macy Cc: Steve Kargl; FreeBSD Current Subject: Re: [PATCH] Recent libm additions On Sun, 2018-07-15 at 11:55 -0600, Warner Losh wrote: > On Sun, Jul 15, 2018, 11:23 AM K. Macy wrote: > > > > > > > > > > > > Well, actually, the functions in polevll.c should have been > > > copied > > > into ld80/e_powl.c, and polevall.c should never have been > > > committed. > > > Unfortunately, the code was not reviewed for correctness. > > That is not correct. Please stop repeating it. Bruce Evans and John > > Baldwin were both looped in. Neither made this observation. > > > Steve is the fp guy these days. And it wasn't reviewed by him. He's > mad you > cut him out of the loop. Arguing about pedantic points of process > does no > one any good. > > Warner On the other hand, what information is there for someone to know that Steve should be involved in a review? There is nothing in MAINTAINERS. The review was on phab for almost a month, and phab is supposedly the preferred way to do reviews these days. Steve is no longer a committer, but that doesn't preclude him having a phab account and participating in reviews. If he doesn't like using phab (and I can certainly understand that POV), an entry in MAINTAINERS would still be helpful, unless we have a rule that only committers can be listed in there. -- Ian ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org" ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On 07/15/2018 02:09 PM, Warner Losh wrote: > I'm not saying that he has a lock. I'm saying he's are domain expert and > many mistakes can be avoided by talking to him. > > I'm saying we have history here, and that history, while poorly documented, > wasn't followed. To the extent it is poorly documented, we should fix that. > > Warner > I agree that we should document the process. Maybe also include freebsd-numerics@ on these discussions, as that is why it was created. But I'm really glad these changes were committed. I have found the people tend to drag their feet a lot on numerics issues. Has anyone done an analysis of the OpenBSD powl functions from an accuracy point of view? That is, to test how many ULP of error these functions have? If not, I could give it a go, although not for several months because life is very busy. ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
So something like this: diff --git a/MAINTAINERS b/MAINTAINERS index 51d3688f8b8..3e6584f24a1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -58,6 +58,7 @@ kqueuejmg Pre-commit review requested. Documentation Required. libdpv dteske Pre-commit review requested. Keep in sync with dpv(1). libfetch des Pre-commit review requested, email only. libfigpar dteske Pre-commit review requested. +libm freebsd-numericsSend email with patches to freebsd-numerics@ libpam des Pre-commit review requested, email only. linprocfs des Pre-commit review requested, email only. lprgad Pre-commit review requested, particularly for is what you're suggesting? Warner On Sun, Jul 15, 2018 at 1:23 PM, Cy Schubert wrote: > I wasn't saying Steve has a lock however in case non-committers might feel > they do, addressing all points in my reply. Not saying anyone feels this > way today but we should consider this in whatever we decide here > (considering all possibilities). IMO adding subject matter experts to > MAINTAINERS seems like the easiest way to document who might be the go-to > person. > > --- > Sent using a tiny phone keyboard. > Apologies for any typos and autocorrect. > Also, this old phone only supports top post. Apologies. > > Cy Schubert > or > The need of the many outweighs the greed of the few. > --- > -- > From: Warner Losh > Sent: 15/07/2018 12:09 > To: Cy Schubert > Cc: Ian Lepore; K. Macy; Steve Kargl; FreeBSD Current > > Subject: Re: [PATCH] Recent libm additions > > I'm not saying that he has a lock. I'm saying he's are domain expert and > many mistakes can be avoided by talking to him. > > I'm saying we have history here, and that history, while poorly > documented, wasn't followed. To the extent it is poorly documented, we > should fix that. > > Warner > > On Sun, Jul 15, 2018 at 12:43 PM, Cy Schubert > wrote: > >> I don't think it makes sense for a non-committer to have a lock on >> anything in base. However a request for review makes a lot of sense. If a >> non-committer or former committer is the SME on a particular subject it's >> best that they be consulted even if they don't request it. IMO more input >> is better. Where better to document this than in MAINTAINERS. >> >> Having said all this. If a person is a former committer and it's not >> documented, how are we to know? >> >> If people agree, should we start documenting SMEs in MAINTAINERS? >> >> --- >> Sent using a tiny phone keyboard. >> Apologies for any typos and autocorrect. >> Also, this old phone only supports top post. Apologies. >> >> Cy Schubert >> or >> The need of the many outweighs the greed of the few. >> --- >> -- >> From: Ian Lepore >> Sent: 15/07/2018 11:08 >> To: Warner Losh; K. Macy >> Cc: Steve Kargl; FreeBSD Current >> Subject: Re: [PATCH] Recent libm additions >> >> On Sun, 2018-07-15 at 11:55 -0600, Warner Losh wrote: >> > On Sun, Jul 15, 2018, 11:23 AM K. Macy wrote: >> > >> > > >> > > > >> > > > >> > > > Well, actually, the functions in polevll.c should have been >> > > > copied >> > > > into ld80/e_powl.c, and polevall.c should never have been >> > > > committed. >> > > > Unfortunately, the code was not reviewed for correctness. >> > > That is not correct. Please stop repeating it. Bruce Evans and John >> > > Baldwin were both looped in. Neither made this observation. >> > > >> > Steve is the fp guy these days. And it wasn't reviewed by him. He's >> > mad you >> > cut him out of the loop. Arguing about pedantic points of process >> > does no >> > one any good. >> > >> > Warner >> >> On the other hand, what information is there for someone to know that >> Steve should be involved in a review? There is nothing in MAINTAINERS. >> The review was on phab for almost a month, and phab is supposedly the >> preferred way to do reviews these days. >> >> Steve is no longer a committer, but that doesn't preclude him having a >> phab account and participating in reviews. If he doesn't like using >> phab (and I can certainly understand that POV), an entry in MAINTAINERS >> would still be helpful, unless we have a rule that only committers can >> be listed in there. >> >> -- Ian >> >> ___ >> freebsd-current@freebsd.org mailing list >> https://lists.freebsd.org/mailman/listinfo/freebsd-current >> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org >> " >> >> > ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
RE: [PATCH] Recent libm additions
I wasn't saying Steve has a lock however in case non-committers might feel they do, addressing all points in my reply. Not saying anyone feels this way today but we should consider this in whatever we decide here (considering all possibilities). IMO adding subject matter experts to MAINTAINERS seems like the easiest way to document who might be the go-to person. --- Sent using a tiny phone keyboard. Apologies for any typos and autocorrect. Also, this old phone only supports top post. Apologies. Cy Schubert or The need of the many outweighs the greed of the few. --- -Original Message- From: Warner Losh Sent: 15/07/2018 12:09 To: Cy Schubert Cc: Ian Lepore; K. Macy; Steve Kargl; FreeBSD Current Subject: Re: [PATCH] Recent libm additions I'm not saying that he has a lock. I'm saying he's are domain expert and many mistakes can be avoided by talking to him. I'm saying we have history here, and that history, while poorly documented, wasn't followed. To the extent it is poorly documented, we should fix that. Warner On Sun, Jul 15, 2018 at 12:43 PM, Cy Schubert wrote: I don't think it makes sense for a non-committer to have a lock on anything in base. However a request for review makes a lot of sense. If a non-committer or former committer is the SME on a particular subject it's best that they be consulted even if they don't request it. IMO more input is better. Where better to document this than in MAINTAINERS. Having said all this. If a person is a former committer and it's not documented, how are we to know? If people agree, should we start documenting SMEs in MAINTAINERS? --- Sent using a tiny phone keyboard. Apologies for any typos and autocorrect. Also, this old phone only supports top post. Apologies. Cy Schubert or The need of the many outweighs the greed of the few. --- From: Ian Lepore Sent: 15/07/2018 11:08 To: Warner Losh; K. Macy Cc: Steve Kargl; FreeBSD Current Subject: Re: [PATCH] Recent libm additions On Sun, 2018-07-15 at 11:55 -0600, Warner Losh wrote: > On Sun, Jul 15, 2018, 11:23 AM K. Macy wrote: > > > > > > > > > > > > Well, actually, the functions in polevll.c should have been > > > copied > > > into ld80/e_powl.c, and polevall.c should never have been > > > committed. > > > Unfortunately, the code was not reviewed for correctness. > > That is not correct. Please stop repeating it. Bruce Evans and John > > Baldwin were both looped in. Neither made this observation. > > > Steve is the fp guy these days. And it wasn't reviewed by him. He's > mad you > cut him out of the loop. Arguing about pedantic points of process > does no > one any good. > > Warner On the other hand, what information is there for someone to know that Steve should be involved in a review? There is nothing in MAINTAINERS. The review was on phab for almost a month, and phab is supposedly the preferred way to do reviews these days. Steve is no longer a committer, but that doesn't preclude him having a phab account and participating in reviews. If he doesn't like using phab (and I can certainly understand that POV), an entry in MAINTAINERS would still be helpful, unless we have a rule that only committers can be listed in there. -- Ian ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org" ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
I'm not saying that he has a lock. I'm saying he's are domain expert and many mistakes can be avoided by talking to him. I'm saying we have history here, and that history, while poorly documented, wasn't followed. To the extent it is poorly documented, we should fix that. Warner On Sun, Jul 15, 2018 at 12:43 PM, Cy Schubert wrote: > I don't think it makes sense for a non-committer to have a lock on > anything in base. However a request for review makes a lot of sense. If a > non-committer or former committer is the SME on a particular subject it's > best that they be consulted even if they don't request it. IMO more input > is better. Where better to document this than in MAINTAINERS. > > Having said all this. If a person is a former committer and it's not > documented, how are we to know? > > If people agree, should we start documenting SMEs in MAINTAINERS? > > --- > Sent using a tiny phone keyboard. > Apologies for any typos and autocorrect. > Also, this old phone only supports top post. Apologies. > > Cy Schubert > or > The need of the many outweighs the greed of the few. > --- > -- > From: Ian Lepore > Sent: 15/07/2018 11:08 > To: Warner Losh; K. Macy > Cc: Steve Kargl; FreeBSD Current > Subject: Re: [PATCH] Recent libm additions > > On Sun, 2018-07-15 at 11:55 -0600, Warner Losh wrote: > > On Sun, Jul 15, 2018, 11:23 AM K. Macy wrote: > > > > > > > > > > > > > > > > > Well, actually, the functions in polevll.c should have been > > > > copied > > > > into ld80/e_powl.c, and polevall.c should never have been > > > > committed. > > > > Unfortunately, the code was not reviewed for correctness. > > > That is not correct. Please stop repeating it. Bruce Evans and John > > > Baldwin were both looped in. Neither made this observation. > > > > > Steve is the fp guy these days. And it wasn't reviewed by him. He's > > mad you > > cut him out of the loop. Arguing about pedantic points of process > > does no > > one any good. > > > > Warner > > On the other hand, what information is there for someone to know that > Steve should be involved in a review? There is nothing in MAINTAINERS. > The review was on phab for almost a month, and phab is supposedly the > preferred way to do reviews these days. > > Steve is no longer a committer, but that doesn't preclude him having a > phab account and participating in reviews. If he doesn't like using > phab (and I can certainly understand that POV), an entry in MAINTAINERS > would still be helpful, unless we have a rule that only committers can > be listed in there. > > -- Ian > > ___ > freebsd-current@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org" > > ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 11:33 AM, Steve Kargl wrote: > On Sun, Jul 15, 2018 at 12:06:47PM -0600, Ian Lepore wrote: >> >> On the other hand, what information is there for someone to know that >> Steve should be involved in a review? There is nothing in MAINTAINERS. >> The review was on phab for almost a month, and phab is supposedly the >> preferred way to do reviews these days. >> >> Steve is no longer a committer, but that doesn't preclude him having a >> phab account and participating in reviews. If he doesn't like using >> phab (and I can certainly understand that POV), an entry in MAINTAINERS >> would still be helpful, unless we have a rule that only committers can >> be listed in there. >> > > Patch should be sent the the freebsd-numeric mailing list. > phab appear after I was forced to hand in my commit bit, so > I don't have a phab account. Anyone can have a phab account. -M ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
RE: [PATCH] Recent libm additions
I don't think it makes sense for a non-committer to have a lock on anything in base. However a request for review makes a lot of sense. If a non-committer or former committer is the SME on a particular subject it's best that they be consulted even if they don't request it. IMO more input is better. Where better to document this than in MAINTAINERS. Having said all this. If a person is a former committer and it's not documented, how are we to know? If people agree, should we start documenting SMEs in MAINTAINERS? --- Sent using a tiny phone keyboard. Apologies for any typos and autocorrect. Also, this old phone only supports top post. Apologies. Cy Schubert or The need of the many outweighs the greed of the few. --- -Original Message- From: Ian Lepore Sent: 15/07/2018 11:08 To: Warner Losh; K. Macy Cc: Steve Kargl; FreeBSD Current Subject: Re: [PATCH] Recent libm additions On Sun, 2018-07-15 at 11:55 -0600, Warner Losh wrote: > On Sun, Jul 15, 2018, 11:23 AM K. Macy wrote: > > > > > > > > > > > > Well, actually, the functions in polevll.c should have been > > > copied > > > into ld80/e_powl.c, and polevall.c should never have been > > > committed. > > > Unfortunately, the code was not reviewed for correctness. > > That is not correct. Please stop repeating it. Bruce Evans and John > > Baldwin were both looped in. Neither made this observation. > > > Steve is the fp guy these days. And it wasn't reviewed by him. He's > mad you > cut him out of the loop. Arguing about pedantic points of process > does no > one any good. > > Warner On the other hand, what information is there for someone to know that Steve should be involved in a review? There is nothing in MAINTAINERS. The review was on phab for almost a month, and phab is supposedly the preferred way to do reviews these days. Steve is no longer a committer, but that doesn't preclude him having a phab account and participating in reviews. If he doesn't like using phab (and I can certainly understand that POV), an entry in MAINTAINERS would still be helpful, unless we have a rule that only committers can be listed in there. -- Ian ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org" ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 10:17 AM, Steve Kargl wrote: > On Sun, Jul 15, 2018 at 11:00:41AM -0600, Ian Lepore wrote: >> On Sun, 2018-07-15 at 08:06 -0700, Steve Kargl wrote: >> > Index: ld80/e_powl.c >> > === >> > --- ld80/e_powl.c (revision 336304) >> > +++ ld80/e_powl.c (working copy) >> > @@ -77,6 +77,7 @@ >> > #include >> > >> > #include "math_private.h" >> > +#include "polevll.c" >> >> If a file contains inline function definitions and is intended only to >> be included into another file and not compiled separately, shouldn't >> its name be spelled polevll.h ? >> > > Well, actually, the functions in polevll.c should have been copied > into ld80/e_powl.c, and polevall.c should never have been committed. > Unfortunately, the code was not reviewed for correctness. I've > made the minimum changes to address the two issues I've noted. > Feel free to either copy the functions and delete the polevall.c > or rename it. > In the bug report you cite, Chris Lattner states: "This is actually an unspecified feature of C99 (whether it supports the _Imaginary type). It is desirable to support this, but not a regression. I'm more than happy to commit these changes, but neither including a .c file nor compensating for the absence of a gcc feature in clang is a correctness fix. In the future you might wish to subscribe to phab reviews so that you can be notified when changes like this are under consideration. Thank you for your input. -M ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 10:44:28AM -0700, Matthew Macy wrote: > > In the bug report you cite, Chris Lattner states: "This is actually an > unspecified feature of C99 (whether it supports the _Imaginary type). > It is desirable to support this, but not a regression. > Chris Lattner is wrong when the use of I in an express gives the wrong answer. He can claim Annex F and G are non-normative, but a wrong answer is still wrong. Go read msun/src/math_private.h. FreeBSD clearly does not use I in libm code, because it has consequences for floating point numerical code. -- Steve ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 12:06:47PM -0600, Ian Lepore wrote: > > On the other hand, what information is there for someone to know that > Steve should be involved in a review? There is nothing in MAINTAINERS. > The review was on phab for almost a month, and phab is supposedly the > preferred way to do reviews these days. > > Steve is no longer a committer, but that doesn't preclude him having a > phab account and participating in reviews. If he doesn't like using > phab (and I can certainly understand that POV), an entry in MAINTAINERS > would still be helpful, unless we have a rule that only committers can > be listed in there. > Patch should be sent the the freebsd-numeric mailing list. phab appear after I was forced to hand in my commit bit, so I don't have a phab account. -- Steve ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 10:21:25AM -0700, K. Macy wrote: > > > > Well, actually, the functions in polevll.c should have been copied > > into ld80/e_powl.c, and polevall.c should never have been committed. > > Unfortunately, the code was not reviewed for correctness. > > That is not correct. Please stop repeating it. Bruce Evans and John > Baldwin were both looped in. Neither made this observation. I read the differential review. The code was not reviewed by John. He reviewed how it was hooked into the build. Bruce does not appear in the differential review. There is no record on freebsd-numerics about the patch. powl on i686-class hardware is likely broken as it does not use the LD80C macro to construct literal constants. -- Steve ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, 2018-07-15 at 11:55 -0600, Warner Losh wrote: > On Sun, Jul 15, 2018, 11:23 AM K. Macy wrote: > > > > > > > > > > > > Well, actually, the functions in polevll.c should have been > > > copied > > > into ld80/e_powl.c, and polevall.c should never have been > > > committed. > > > Unfortunately, the code was not reviewed for correctness. > > That is not correct. Please stop repeating it. Bruce Evans and John > > Baldwin were both looped in. Neither made this observation. > > > Steve is the fp guy these days. And it wasn't reviewed by him. He's > mad you > cut him out of the loop. Arguing about pedantic points of process > does no > one any good. > > Warner On the other hand, what information is there for someone to know that Steve should be involved in a review? There is nothing in MAINTAINERS. The review was on phab for almost a month, and phab is supposedly the preferred way to do reviews these days. Steve is no longer a committer, but that doesn't preclude him having a phab account and participating in reviews. If he doesn't like using phab (and I can certainly understand that POV), an entry in MAINTAINERS would still be helpful, unless we have a rule that only committers can be listed in there. -- Ian ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 10:55 AM, Warner Losh wrote: > On Sun, Jul 15, 2018, 11:23 AM K. Macy wrote: > >> > >> > Well, actually, the functions in polevll.c should have been copied >> > into ld80/e_powl.c, and polevall.c should never have been committed. >> > Unfortunately, the code was not reviewed for correctness. >> >> That is not correct. Please stop repeating it. Bruce Evans and John >> Baldwin were both looped in. Neither made this observation. >> > > Steve is the fp guy these days. And it wasn't reviewed by him. He's mad you > cut him out of the loop. Arguing about pedantic points of process does no > one any good. Thanks for the tip. I'm sorry. I was under the impression that he gave up his bit: https://reviews.freebsd.org/rD46886 So we have a maintainer who has opted to not have a bit. So be it. Nonetheless, reviews.freebsd.org is the established channel by which the project does code reviews. I stand by my recommendation and will add him to reviews in the future. ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018, 11:23 AM K. Macy wrote: > > > > Well, actually, the functions in polevll.c should have been copied > > into ld80/e_powl.c, and polevall.c should never have been committed. > > Unfortunately, the code was not reviewed for correctness. > > That is not correct. Please stop repeating it. Bruce Evans and John > Baldwin were both looped in. Neither made this observation. > Steve is the fp guy these days. And it wasn't reviewed by him. He's mad you cut him out of the loop. Arguing about pedantic points of process does no one any good. Warner ___ > freebsd-current@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org" > ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
> > Well, actually, the functions in polevll.c should have been copied > into ld80/e_powl.c, and polevall.c should never have been committed. > Unfortunately, the code was not reviewed for correctness. That is not correct. Please stop repeating it. Bruce Evans and John Baldwin were both looped in. Neither made this observation. ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, Jul 15, 2018 at 11:00:41AM -0600, Ian Lepore wrote: > On Sun, 2018-07-15 at 08:06 -0700, Steve Kargl wrote: > > Index: ld80/e_powl.c > > === > > --- ld80/e_powl.c (revision 336304) > > +++ ld80/e_powl.c (working copy) > > @@ -77,6 +77,7 @@ > > #include > > > > #include "math_private.h" > > +#include "polevll.c" > > If a file contains inline function definitions and is intended only to > be included into another file and not compiled separately, shouldn't > its name be spelled polevll.h ? > Well, actually, the functions in polevll.c should have been copied into ld80/e_powl.c, and polevall.c should never have been committed. Unfortunately, the code was not reviewed for correctness. I've made the minimum changes to address the two issues I've noted. Feel free to either copy the functions and delete the polevall.c or rename it. -- Steve ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
On Sun, 2018-07-15 at 08:06 -0700, Steve Kargl wrote: > Apparently, the recents additions to libm were not > subject to any code review. The following patch > does two things. First, it works around > > https://bugs.llvm.org/show_bug.cgi?id=8532 > > Second, it removes the pollution of libm with the > polevll.c functions. Those functions are used > only in ld80/e_powl.c, and those functions should > be inlined. > > Index: Makefile > === > --- Makefile (revision 336304) > +++ Makefile (working copy) > @@ -56,7 +56,6 @@ > imprecise.c \ > k_cos.c k_cosf.c k_exp.c k_expf.c k_rem_pio2.c k_sin.c k_sinf.c \ > k_tan.c k_tanf.c \ > - polevll.c \ > s_asinh.c s_asinhf.c s_atan.c s_atanf.c s_carg.c s_cargf.c s_cargl.c \ > s_cbrt.c s_cbrtf.c s_ceil.c s_ceilf.c s_clog.c s_clogf.c \ > s_copysign.c s_copysignf.c s_cos.c s_cosf.c \ > Index: ld80/e_powl.c > === > --- ld80/e_powl.c (revision 336304) > +++ ld80/e_powl.c (working copy) > @@ -77,6 +77,7 @@ > #include > > #include "math_private.h" > +#include "polevll.c" > > /* Table size */ > #define NXT 32 > Index: src/math_private.h > === > --- src/math_private.h(revision 336304) > +++ src/math_private.h(working copy) > @@ -828,7 +828,4 @@ > long double __kernel_cosl(long double, long double); > long double __kernel_tanl(long double, long double, int); > > -long double __p1evll(long double, void *, int); > -long double __polevll(long double, void *, int); > - > #endif /* !_MATH_PRIVATE_H_ */ > Index: src/polevll.c > === > --- src/polevll.c (revision 336304) > +++ src/polevll.c (working copy) > @@ -69,7 +69,7 @@ > * Polynomial evaluator: > * P[0] x^n + P[1] x^(n-1) + ... + P[n] > */ > -long double > +static inline long double > __polevll(long double x, void *PP, int n) > { > long double y; > @@ -88,7 +88,7 @@ > * Polynomial evaluator: > * x^n + P[0] x^(n-1) + P[1] x^(n-2) + ... + P[n] > */ > -long double > +static inline long double > __p1evll(long double x, void *PP, int n) > { > long double y; > Index: src/s_cpow.c > === > --- src/s_cpow.c (revision 336304) > +++ src/s_cpow.c (working copy) > @@ -60,7 +60,7 @@ > y = cimag (z); > absa = cabs (a); > if (absa == 0.0) { > - return (0.0 + 0.0 * I); > + return (CMPLX(0.0, 0.0)); > } > arga = carg (a); > r = pow (absa, x); > @@ -69,6 +69,6 @@ > r = r * exp (-y * arga); > theta = theta + y * log (absa); > } > - w = r * cos (theta) + (r * sin (theta)) * I; > + w = CMPLX(r * cos (theta), r * sin (theta)); > return (w); > } > Index: src/s_cpowf.c > === > --- src/s_cpowf.c (revision 336304) > +++ src/s_cpowf.c (working copy) > @@ -59,7 +59,7 @@ > y = cimagf(z); > absa = cabsf (a); > if (absa == 0.0f) { > - return (0.0f + 0.0f * I); > + return (CMPLXF(0.0f, 0.0f)); > } > arga = cargf (a); > r = powf (absa, x); > @@ -68,6 +68,6 @@ > r = r * expf (-y * arga); > theta = theta + y * logf (absa); > } > - w = r * cosf (theta) + (r * sinf (theta)) * I; > + w = CMPLXF(r * cosf (theta), r * sinf (theta)); > return (w); > } > Index: src/s_cpowl.c > === > --- src/s_cpowl.c (revision 336304) > +++ src/s_cpowl.c (working copy) > @@ -59,7 +59,7 @@ > y = cimagl(z); > absa = cabsl(a); > if (absa == 0.0L) { > - return (0.0L + 0.0L * I); > + return (CMPLXL(0.0L, 0.0L)); > } > arga = cargl(a); > r = powl(absa, x); > @@ -68,6 +68,6 @@ > r = r * expl(-y * arga); > theta = theta + y * logl(absa); > } > - w = r * cosl(theta) + (r * sinl(theta)) * I; > + w = CMPLXL(r * cosl(theta), r * sinl(theta)); > return (w); > } > If a file contains inline function definitions and is intended only to be included into another file and not compiled separately, shouldn't its name be spelled polevll.h ? -- Ian ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [PATCH] Recent libm additions
These changes look perfect to me. Warner On Sun, Jul 15, 2018, 9:08 AM Steve Kargl wrote: > Apparently, the recents additions to libm were not > subject to any code review. The following patch > does two things. First, it works around > > https://bugs.llvm.org/show_bug.cgi?id=8532 > > Second, it removes the pollution of libm with the > polevll.c functions. Those functions are used > only in ld80/e_powl.c, and those functions should > be inlined. > > Index: Makefile > === > --- Makefile(revision 336304) > +++ Makefile(working copy) > @@ -56,7 +56,6 @@ > imprecise.c \ > k_cos.c k_cosf.c k_exp.c k_expf.c k_rem_pio2.c k_sin.c k_sinf.c \ > k_tan.c k_tanf.c \ > - polevll.c \ > s_asinh.c s_asinhf.c s_atan.c s_atanf.c s_carg.c s_cargf.c > s_cargl.c \ > s_cbrt.c s_cbrtf.c s_ceil.c s_ceilf.c s_clog.c s_clogf.c \ > s_copysign.c s_copysignf.c s_cos.c s_cosf.c \ > Index: ld80/e_powl.c > === > --- ld80/e_powl.c (revision 336304) > +++ ld80/e_powl.c (working copy) > @@ -77,6 +77,7 @@ > #include > > #include "math_private.h" > +#include "polevll.c" > > /* Table size */ > #define NXT 32 > Index: src/math_private.h > === > --- src/math_private.h (revision 336304) > +++ src/math_private.h (working copy) > @@ -828,7 +828,4 @@ > long double __kernel_cosl(long double, long double); > long double __kernel_tanl(long double, long double, int); > > -long double __p1evll(long double, void *, int); > -long double __polevll(long double, void *, int); > - > #endif /* !_MATH_PRIVATE_H_ */ > Index: src/polevll.c > === > --- src/polevll.c (revision 336304) > +++ src/polevll.c (working copy) > @@ -69,7 +69,7 @@ > * Polynomial evaluator: > * P[0] x^n + P[1] x^(n-1) + ... + P[n] > */ > -long double > +static inline long double > __polevll(long double x, void *PP, int n) > { > long double y; > @@ -88,7 +88,7 @@ > * Polynomial evaluator: > * x^n + P[0] x^(n-1) + P[1] x^(n-2) + ... + P[n] > */ > -long double > +static inline long double > __p1evll(long double x, void *PP, int n) > { > long double y; > Index: src/s_cpow.c > === > --- src/s_cpow.c(revision 336304) > +++ src/s_cpow.c(working copy) > @@ -60,7 +60,7 @@ > y = cimag (z); > absa = cabs (a); > if (absa == 0.0) { > - return (0.0 + 0.0 * I); > + return (CMPLX(0.0, 0.0)); > } > arga = carg (a); > r = pow (absa, x); > @@ -69,6 +69,6 @@ > r = r * exp (-y * arga); > theta = theta + y * log (absa); > } > - w = r * cos (theta) + (r * sin (theta)) * I; > + w = CMPLX(r * cos (theta), r * sin (theta)); > return (w); > } > Index: src/s_cpowf.c > === > --- src/s_cpowf.c (revision 336304) > +++ src/s_cpowf.c (working copy) > @@ -59,7 +59,7 @@ > y = cimagf(z); > absa = cabsf (a); > if (absa == 0.0f) { > - return (0.0f + 0.0f * I); > + return (CMPLXF(0.0f, 0.0f)); > } > arga = cargf (a); > r = powf (absa, x); > @@ -68,6 +68,6 @@ > r = r * expf (-y * arga); > theta = theta + y * logf (absa); > } > - w = r * cosf (theta) + (r * sinf (theta)) * I; > + w = CMPLXF(r * cosf (theta), r * sinf (theta)); > return (w); > } > Index: src/s_cpowl.c > === > --- src/s_cpowl.c (revision 336304) > +++ src/s_cpowl.c (working copy) > @@ -59,7 +59,7 @@ > y = cimagl(z); > absa = cabsl(a); > if (absa == 0.0L) { > - return (0.0L + 0.0L * I); > + return (CMPLXL(0.0L, 0.0L)); > } > arga = cargl(a); > r = powl(absa, x); > @@ -68,6 +68,6 @@ > r = r * expl(-y * arga); > theta = theta + y * logl(absa); > } > - w = r * cosl(theta) + (r * sinl(theta)) * I; > + w = CMPLXL(r * cosl(theta), r * sinl(theta)); > return (w); > } > > -- > Steve > ___ > freebsd-current@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org" > ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
[PATCH] Recent libm additions
Apparently, the recents additions to libm were not subject to any code review. The following patch does two things. First, it works around https://bugs.llvm.org/show_bug.cgi?id=8532 Second, it removes the pollution of libm with the polevll.c functions. Those functions are used only in ld80/e_powl.c, and those functions should be inlined. Index: Makefile === --- Makefile(revision 336304) +++ Makefile(working copy) @@ -56,7 +56,6 @@ imprecise.c \ k_cos.c k_cosf.c k_exp.c k_expf.c k_rem_pio2.c k_sin.c k_sinf.c \ k_tan.c k_tanf.c \ - polevll.c \ s_asinh.c s_asinhf.c s_atan.c s_atanf.c s_carg.c s_cargf.c s_cargl.c \ s_cbrt.c s_cbrtf.c s_ceil.c s_ceilf.c s_clog.c s_clogf.c \ s_copysign.c s_copysignf.c s_cos.c s_cosf.c \ Index: ld80/e_powl.c === --- ld80/e_powl.c (revision 336304) +++ ld80/e_powl.c (working copy) @@ -77,6 +77,7 @@ #include #include "math_private.h" +#include "polevll.c" /* Table size */ #define NXT 32 Index: src/math_private.h === --- src/math_private.h (revision 336304) +++ src/math_private.h (working copy) @@ -828,7 +828,4 @@ long double __kernel_cosl(long double, long double); long double __kernel_tanl(long double, long double, int); -long double __p1evll(long double, void *, int); -long double __polevll(long double, void *, int); - #endif /* !_MATH_PRIVATE_H_ */ Index: src/polevll.c === --- src/polevll.c (revision 336304) +++ src/polevll.c (working copy) @@ -69,7 +69,7 @@ * Polynomial evaluator: * P[0] x^n + P[1] x^(n-1) + ... + P[n] */ -long double +static inline long double __polevll(long double x, void *PP, int n) { long double y; @@ -88,7 +88,7 @@ * Polynomial evaluator: * x^n + P[0] x^(n-1) + P[1] x^(n-2) + ... + P[n] */ -long double +static inline long double __p1evll(long double x, void *PP, int n) { long double y; Index: src/s_cpow.c === --- src/s_cpow.c(revision 336304) +++ src/s_cpow.c(working copy) @@ -60,7 +60,7 @@ y = cimag (z); absa = cabs (a); if (absa == 0.0) { - return (0.0 + 0.0 * I); + return (CMPLX(0.0, 0.0)); } arga = carg (a); r = pow (absa, x); @@ -69,6 +69,6 @@ r = r * exp (-y * arga); theta = theta + y * log (absa); } - w = r * cos (theta) + (r * sin (theta)) * I; + w = CMPLX(r * cos (theta), r * sin (theta)); return (w); } Index: src/s_cpowf.c === --- src/s_cpowf.c (revision 336304) +++ src/s_cpowf.c (working copy) @@ -59,7 +59,7 @@ y = cimagf(z); absa = cabsf (a); if (absa == 0.0f) { - return (0.0f + 0.0f * I); + return (CMPLXF(0.0f, 0.0f)); } arga = cargf (a); r = powf (absa, x); @@ -68,6 +68,6 @@ r = r * expf (-y * arga); theta = theta + y * logf (absa); } - w = r * cosf (theta) + (r * sinf (theta)) * I; + w = CMPLXF(r * cosf (theta), r * sinf (theta)); return (w); } Index: src/s_cpowl.c === --- src/s_cpowl.c (revision 336304) +++ src/s_cpowl.c (working copy) @@ -59,7 +59,7 @@ y = cimagl(z); absa = cabsl(a); if (absa == 0.0L) { - return (0.0L + 0.0L * I); + return (CMPLXL(0.0L, 0.0L)); } arga = cargl(a); r = powl(absa, x); @@ -68,6 +68,6 @@ r = r * expl(-y * arga); theta = theta + y * logl(absa); } - w = r * cosl(theta) + (r * sinl(theta)) * I; + w = CMPLXL(r * cosl(theta), r * sinl(theta)); return (w); } -- Steve ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"