Remove log2 define

Message ID DB5PR08MB10303550663ED2755CF4880683360@DB5PR08MB1030.eurprd08.prod.outlook.com
State New
Headers show
Series
  • Remove log2 define
Related show

Commit Message

Wilco Dijkstra Aug. 24, 2018, 2:37 p.m.
Since newlib now has a fast implementation of log2, remove a define which
avoids calling log2.  The define makes math code slower and less accurate,
so removing it is best.

Build OK on AArch64, OK for commit?

--

Comments

Corinna Vinschen Aug. 24, 2018, 2:45 p.m. | #1
On Aug 24 14:37, Wilco Dijkstra wrote:
> Since newlib now has a fast implementation of log2,


Only if !__OBSOLETE_MATH

> remove a define which

> avoids calling log2.  The define makes math code slower and less accurate,

> so removing it is best.

> 

> Build OK on AArch64, OK for commit?

> 

> --

> diff --git a/newlib/libc/include/math.h b/newlib/libc/include/math.h

> index 893a5d0645148af0dd0a48f7e73504e07363a840..2d17dd13e52ae1893beba8c9a2565d7682dbbf89 100644

> --- a/newlib/libc/include/math.h

> +++ b/newlib/libc/include/math.h

> @@ -329,9 +329,6 @@ extern double lgamma (double);

>  extern double erf (double);

>  extern double erfc (double);

>  extern double log2 (double);

> -#if !defined(__cplusplus)

> -#define log2(x) (log (x) / _M_LN2)

> -#endif

>  

>  #ifndef __math_68881

>  extern double hypot (double, double);


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
Wilco Dijkstra Aug. 24, 2018, 4:04 p.m. | #2
Corinna Vinschen wrote:

>> Since newlib now has a fast implementation of log2,

>

> Only if !__OBSOLETE_MATH


Is there any reason keep that old code, particularly for the rewritten double
precision functions? We now have a log2 that is both fast, accurate and
correct, which isn't true for the obsolete version.

Wilco
Corinna Vinschen Aug. 27, 2018, 10:44 a.m. | #3
On Aug 24 16:04, Wilco Dijkstra wrote:
> Corinna Vinschen wrote:

> 

> >> Since newlib now has a fast implementation of log2,

> >

> > Only if !__OBSOLETE_MATH

> 

> Is there any reason keep that old code, particularly for the rewritten double

> precision functions? We now have a log2 that is both fast, accurate and

> correct, which isn't true for the obsolete version.


If you want to make it default for all targets, I guess you can go
ahead and send patches to remove the obsolete code.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
Tamar Christina Sept. 5, 2018, 11:54 a.m. | #4
> -----Original Message-----

> From: newlib-owner@sourceware.org <newlib-owner@sourceware.org> On

> Behalf Of Corinna Vinschen

> Sent: Monday, August 27, 2018 11:45

> To: Wilco Dijkstra <Wilco.Dijkstra@arm.com>

> Cc: newlib@sourceware.org; nd <nd@arm.com>

> Subject: Re: [PATCH] Remove log2 define

> 

> On Aug 24 16:04, Wilco Dijkstra wrote:

> > Corinna Vinschen wrote:

> >

> > >> Since newlib now has a fast implementation of log2,

> > >

> > > Only if !__OBSOLETE_MATH

> >

> > Is there any reason keep that old code, particularly for the rewritten

> > double precision functions? We now have a log2 that is both fast,

> > accurate and correct, which isn't true for the obsolete version.

> 

> If you want to make it default for all targets, I guess you can go ahead and

> send patches to remove the obsolete code.

> 


Should that action actually block this one? We've had cases where this optimization isn't safe
when the user compiles this header and their code with fast-math.  So I'd say there's an actual
bug here as well.

Regards,
Tamar.

> Corinna

> 

> --

> Corinna Vinschen

> Cygwin Maintainer

> Red Hat

Patch

diff --git a/newlib/libc/include/math.h b/newlib/libc/include/math.h
index 893a5d0645148af0dd0a48f7e73504e07363a840..2d17dd13e52ae1893beba8c9a2565d7682dbbf89 100644
--- a/newlib/libc/include/math.h
+++ b/newlib/libc/include/math.h
@@ -329,9 +329,6 @@  extern double lgamma (double);
 extern double erf (double);
 extern double erfc (double);
 extern double log2 (double);
-#if !defined(__cplusplus)
-#define log2(x) (log (x) / _M_LN2)
-#endif
 
 #ifndef __math_68881
 extern double hypot (double, double);