This is the mail archive of the 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]

[PATCH] Fix lrint, lrintf bugs

  Hi all, here's take 2.  I'm happy with this one now but comments etc.


  I decided that when the exponent was -1, there was still a virtual '1' bit
in the 0.5 position which might need taking into account in certain rounding
modes, so testing for less than or equal to minus one was wrong.  Instead, I
added code to detect the all-zeros representations of plus and minus zero,
and short-circuit the calculation that would otherwise fail due to
miscalculated shift amounts invoking undefined behaviour ("result = i0 >>

  While I was in there, I made the comments consistent between the two
(modulo fp type size differences) just as a tidy up.

  I also added casts to fix the signed-vs-unsigned comparison problem.  This
does generate a warning if you compile the file with -W:

Admin@ubik /usr/build/obj> gcc -L/usr/build/obj/i686-pc-cygwin/winsup [ ...
] -DPACKAGE=\"newlib\" -DVERSION=\"1.13.0\"  -I. -I/usr/build/src/newlib/l
LIB -DHAVE_FCNTL -DMALLOC_PROVIDED -fno-builtin      -O2 -g -O2 -c
rc/newlib/libm/common/s_lrint.c -W
/usr/build/src/newlib/libm/common/s_lrint.c: In function `lrint':
/usr/build/src/newlib/libm/common/s_lrint.c:83: warning: comparison between
ed and unsigned

but that's not the default for newlib, at least in cygwin builds.

  I tested the fix with the attached C file and shell script.  It exercises
both functions across a range of values in both cygwin (which uses newlib)
and mingw (which uses inline x87 asm intrinsics); I believe the mingw
implementation to be correct, and after this patch newlib matches it, with
one exception - the mingw compiler apparently fails to generate the correct
FP constant for minus zero - well, either that or the mingw printf fails to
distinguish between them, I haven't checked yet as it's beside the point!

  Um.  If I'm brutally honest I suppose I should point out that the testing
doesn't cover very large values.  I'll do that offline after I send this
post.  In the meantime, assume this is good unless I report back it went

Can't think of a witty .sigline today....

Attachment: lr.c
Description: Text document

Attachment: lrintf-patch.diff
Description: Binary data

Description: Binary data

Attachment: lrtest-after.txt
Description: Text document

Attachment: lrtest-before.txt
Description: Text document

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