Re: [PATCH] Recent libm additions

2018-07-18 Thread Steve Kargl
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 * 

Re: [PATCH] Recent libm additions

2018-07-16 Thread Steve Kargl
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

2018-07-16 Thread Dima Pasechnik
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

2018-07-15 Thread Mark Linimon
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

2018-07-15 Thread Matthew Macy
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

2018-07-15 Thread Steve Kargl
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

2018-07-15 Thread Steve Kargl
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

2018-07-15 Thread Steve Kargl
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

2018-07-15 Thread Cy Schubert
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

2018-07-15 Thread Montgomery-Smith, Stephen
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

2018-07-15 Thread Warner Losh
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

2018-07-15 Thread Cy Schubert
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

2018-07-15 Thread Warner Losh
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

2018-07-15 Thread Matthew Macy
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

2018-07-15 Thread Cy Schubert
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

2018-07-15 Thread Matthew Macy
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

2018-07-15 Thread Steve Kargl
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

2018-07-15 Thread Steve Kargl
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

2018-07-15 Thread Steve Kargl
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

2018-07-15 Thread Ian Lepore
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

2018-07-15 Thread Matthew Macy
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

2018-07-15 Thread Warner Losh
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

2018-07-15 Thread K. Macy
>
> 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

2018-07-15 Thread Steve Kargl
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

2018-07-15 Thread Ian Lepore
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

2018-07-15 Thread Warner Losh
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"