This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [libcxx+newlib] nexttoward{f,l,} shims for _LDBL_EQ_DBL systems


Sigh, trying again with less context in the diff. Apparently the mailer daemon doesn't like big patches.


Jon

On 12/2/14 4:58 PM, Jonathan Roelofs wrote:
JF reminds me that I forgot to attach the patch... oops.

Jon

On 12/2/14 4:45 PM, Jonathan Roelofs wrote:
Here's a rebased version with the broken nexttowardf replaced by the one from
musl.


Cheers,

Jon

On 12/2/14 11:55 AM, Jonathan Roelofs wrote:
JF,

No. I've been told by one of my coworkers that nexttowardf cannot be implemented
correctly on top of nextafterf. For that one we'll need a different
implementation, perhaps we can copy the one in musl?... I'm not sure if the
license is compatible with doing that.

Everything in this patch aside from the nexttowardf implementation is ready to
go as far as I'm concerned. Craig, how about on your end?


Cheers,

Jon

On 12/2/14 11:45 AM, JF Bastien wrote:
Is this change ready to land?

On Wed, Sep 24, 2014 at 9:38 AM, Jonathan Roelofs
<jonathan@codesourcery.com> wrote:
Craig,

Thanks for the review!

On 9/23/14 7:17 PM, Craig Howland wrote:

On 09/23/2014 04:33 PM, Jonathan Roelofs wrote:

Please see the attached patch.

Needs a ChangeLog entry.  Missing the patch for libm/common/Makefile.am.

Oops. I'm not very familiar with Newlib processes, so please let me know if
I'm missing other things that you'd normally expect from such an upstream
submission.... For example, some of my co-workers reminded me to also update
COPYING.NEWLIB, and add copyright headers to the new files.


Operationally, all looks OK, but aesthetically, the use of casting is
inconsistent.  For example:

+long double
+nexttowardl (long double x, long double y)
+{
+  return nextafter((double)x, (double)y);
+}

The x and y arguments are cast to double to pass to nextafter(), but the
return
value is not cast to long double.  It works OK, but could be confusing.
Are the
casts intended to highlight the spots where the double/long double
equality is
required?

Yeah, that was my intent.

If so, it would probably be better to either cast in all places where
it is theoretically needed, or to just leave them out.  The other existing
functions of this style left all of them out (i.e. no casts from ld to d
or d to
ld)--see, for example, libm/common/nextafterl.c.  I suggest that the
precedent
is the better way.

Agreed, consistency is probably better.

Attached is a new version of this patch with these things addressed, and
I've added another similar shim for log2l.


Cheers,

Jon


Craig


--
Jon Roelofs
jonathan@codesourcery.com
CodeSourcery / Mentor Embedded




--
Jon Roelofs
jonathan@codesourcery.com
CodeSourcery / Mentor Embedded

Attachment: newlib_nexttoward4.diff
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]