Tag a new release - 0.5 #117

ViralBShah · 2016-02-23T19:00:21Z

With the ppc support, I think we should tag a new release - 0.5. Is there anything else we need to do before we tag?

tkelman · 2016-02-23T19:22:24Z

We should probably run julia's tests against it on a variety of platforms.

ViralBShah · 2016-02-23T19:28:46Z

What's the best way to do so? The changes here are minor and there are no api changes - so I think this is pretty safe.

StefanKarpinski · 2016-02-23T19:37:12Z

Is it likely that there will be and API changes in the future? If not, then a 1.0 release may be warranted. Bugfixes would go into 1.0.x releases; new exported functions would go in 1.x releases, and any breaking API changes (including deleting exported functions) would require a 2.0 release.

tkelman · 2016-02-23T19:45:14Z

What's the best way to do so?

Prepare the Julia PR that changes the openlibm.version and checksum files first, run it through CI before doing the tag here?

Is it likely that there will be and API changes in the future?

Not necessarily API changes, but in terms of code #108 will be pretty big and still needs some cleanup. I also think we should look carefully at what musl has done with their libm since they started from pretty much the same code bases we did, but have been better at maintaining and pulling in bug fixes.

ViralBShah · 2016-02-23T19:48:57Z

Can you point to the musl implementation? The issue we have faced is that we never automated the migration from msun. It needs a bunch of sed scripts really, and unfortunately I didn't save that in #108. I'll have to do it once more carefully.

ViralBShah · 2016-02-23T19:49:55Z

By definition, there should be no major API changes, since the goal is to be a libm replacement. I would like to sort out #108 and keeping up with msun before tagging 1.0.

tkelman · 2016-02-23T19:52:31Z

http://git.musl-libc.org/cgit/musl/tree/src/math - looks like they don't have powerpc support (edit: at least not as optimized assembly) at the moment though

ViralBShah · 2016-02-23T19:56:43Z

That code looks quite clean! Do they maintain a record of what all they pull in from msun and how frequently?

tkelman · 2016-02-23T20:01:24Z

http://git.musl-libc.org/cgit/musl/log/src/math

ViralBShah · 2016-02-23T20:08:37Z

I meant tracking the msun logs. Looks like it started from msun, but has significantly changed, and ongoing fixes are manually tracked. I'll try this out to see if there is merit in following musl instead of msun before refreshing msun.

ViralBShah · 2016-02-23T20:15:42Z

Detailed notes on the musl approach - http://nsz.repo.hu/libm/

Many functions have been completely rewritten in musl since the original checkin.

tkelman · 2016-02-23T21:52:05Z

The numpy maintainers have also brought up bionic's libm, used on android: https://github.com/android/platform_bionic/tree/master/libm

ViralBShah · 2016-02-24T02:15:40Z

Bionic looks nice in that it does closely follow msun.

ViralBShah · 2016-02-24T03:12:05Z

I think at the very least, we should follow the bionic model here, and maintain a set of patches external to msun that we need in openlibm. With that we can be 1.0.

I love what the musl guys have done, but need to check with them on long term maintenance plans and understand the community momentum, before we adopt that as the way forward. What Szabolcs Nagy has done there, is what I wanted to do with openlibm in the first place, but he's done really amazing stuff.

nalimilan · 2016-02-24T11:05:25Z

I've just checked that building a RPM works with GCC6, and that's OK. But tests fail on i686:

+ make -C test test-float
make: Entering directory '/builddir/build/BUILD/openlibm-master/test'
gcc  -O2   test-float.c -D__BSD_VISIBLE -I ../include -I../src -L.. -lopenlibm -Wl,-rpath=/builddir/build/BUILD/openlibm-master -o test-float
../libopenlibm.so: undefined reference to `fpsetprec'
../libopenlibm.so: undefined reference to `fpgetprec'
collect2: error: ld returned 1 exit status
Makefile:18: recipe for target 'test-float' failed

test-float used to pass on i686, though test-double failed. (Tests also fail on ARMv7 but that was already the case with 0.4.)

Also, do we actually need to bump the SOVERSION to 2? Did the API really break?

ViralBShah · 2016-02-24T11:15:09Z

I did not bump the SOVERSION here, but I do believe it was required when we removed some APIs.

I think the test failure on ARM is now a couple of minor things.

tkelman · 2016-02-24T16:50:46Z

the undefined references also occurred on win32 appveyor for the test pr, and win64 errored due to incorrect dllexport

ViralBShah · 2016-02-27T05:11:24Z

@tkelman Could you turn on appveyor? I logged into appveyor and authorized it to access openlibm and kicked off a build? But, I guess we need an appveyor.yml file and some other things to be done to turn this on for every commit and PR?

ViralBShah · 2016-02-27T07:22:32Z

The i686 build is fixed. Can you try again? Should work on windows too, but would be nice to setup appveyor.

tkelman · 2016-02-27T08:06:51Z

There isn't currently a template for doing source builds with gcc on appveyor, it takes a fair bit of work to obtain the right toolchain and run from within msys2 or cygwin, it's not a 5-minute thing.

ViralBShah · 2016-02-27T08:45:27Z

Ok. Then I think this is good to tag for now, and we can make further releases as we find issues.

tkelman · 2016-02-27T08:47:26Z

For now there should be an option to "only build when appveyor.yml is present" that you can turn on to avoid the red status until we figure that out.

ViralBShah · 2016-02-27T08:50:59Z

I checked Skip branches without appveyor.yml

tkelman · 2016-02-27T09:34:25Z

You can rebase JuliaLang/julia#15212 for now to see if it works on Windows within Julia's build system - I think there are still dllexport problems that need to be fixed before tagging. AppVeyor here standalone would have to look like a cut-down version of Julia's appveyor setup, which is kind of complicated. I actually am not sure whether openlibm's makefiles work properly if you try to build them on Windows by themselves, outside of Julia's build system.

tkelman · 2016-02-27T10:29:06Z

At least locally it's working on win32 but failing on win64, see the appveyor log from that PR for the dllexport-related error. Bisect points to 8b3b520 as being the first bad commit.

ViralBShah · 2016-02-27T11:22:45Z

I merged #118 and verified that all tests pass on 32-bit and 64-bit linux, as well as OS-X is the same level of tests passing as before. On ARM 32-bit, which used to hang/crash, the test suite now runs completely, with a few FP exception errors. If win32 and win64 are looking good, I think this is a go for 0.5.

nalimilan · 2016-02-27T11:28:58Z

Wait for a bit, I'm running the build.

nalimilan · 2016-02-27T11:43:04Z

Looks good, thanks! Also good to see ARM tests closer to passing completely.

If you want a perfect release, you can have a look at these warnings:

gcc  -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -fno-gnu89-inline -std=c99 -Wall -I/builddir/build/BUILD/openlibm-master -I/builddir/build/BUILD/openlibm-master/include -I/builddir/build/BUILD/openlibm-master/ld80 -I/builddir/build/BUILD/openlibm-master/amd64 -I/builddir/build/BUILD/openlibm-master/src -DASSEMBLER -D__BSD_VISIBLE -Wno-implicit-function-declaration -fPIC -c ld80/s_exp2l.c -o ld80/s_exp2l.c.o
In file included from ld80/s_exp2l.c:33:0:
/builddir/build/BUILD/openlibm-master/amd64/bsd_cdefs.h:129:0: warning: "__P" redefined
 #define __P(protos) protos  /* full-blown ANSI C */

In file included from /usr/include/features.h:368:0,
                 from /usr/include/stdio.h:27,
                 from /builddir/build/BUILD/openlibm-master/src/cdefs-compat.h:12,
                 from ld80/s_exp2l.c:27:
/usr/include/sys/cdefs.h:82:0: note: this is the location of the previous definition
 #define __P(args) args

In file included from ld80/s_exp2l.c:33:0:
/builddir/build/BUILD/openlibm-master/amd64/bsd_cdefs.h:131:0: warning: "__CONCAT" redefined
 #define __CONCAT(x,y) __CONCAT1(x,y)

In file included from /usr/include/features.h:368:0,
                 from /usr/include/stdio.h:27,
                 from /builddir/build/BUILD/openlibm-master/src/cdefs-compat.h:12,
                 from ld80/s_exp2l.c:27:
/usr/include/sys/cdefs.h:88:0: note: this is the location of the previous definition
 #define __CONCAT(x,y) x ## y

In file included from ld80/s_exp2l.c:33:0:
/builddir/build/BUILD/openlibm-master/amd64/bsd_cdefs.h:230:0: warning: "__always_inline" redefined
 #define __always_inline __attribute__((__always_inline__))

In file included from /usr/include/features.h:368:0,
                 from /usr/include/stdio.h:27,
                 from /builddir/build/BUILD/openlibm-master/src/cdefs-compat.h:12,
                 from ld80/s_exp2l.c:27:
/usr/include/sys/cdefs.h:307:0: note: this is the location of the previous definition
 # define __always_inline __inline __attribute__ ((__always_inline__))

In file included from ld80/s_exp2l.c:33:0:
/builddir/build/BUILD/openlibm-master/amd64/bsd_cdefs.h:242:0: warning: "__nonnull" redefined
 #define __nonnull(x) __attribute__((__nonnull__(x)))

In file included from /usr/include/features.h:368:0,
                 from /usr/include/stdio.h:27,
                 from /builddir/build/BUILD/openlibm-master/src/cdefs-compat.h:12,
                 from ld80/s_exp2l.c:27:
/usr/include/sys/cdefs.h:285:0: note: this is the location of the previous definition
 # define __nonnull(params) __attribute__ ((__nonnull__ params))

In file included from ld80/s_exp2l.c:33:0:
/builddir/build/BUILD/openlibm-master/amd64/bsd_cdefs.h:361:0: warning: "__strong_reference" redefined
 #define __strong_reference(sym,aliassym) \

In file included from ld80/s_exp2l.c:27:0:
/builddir/build/BUILD/openlibm-master/src/cdefs-compat.h:28:0: note: this is the location of the previous definition
 #define __strong_reference(sym,aliassym) \

In file included from /builddir/build/BUILD/openlibm-master/src/math_private.h:23:0,
                 from ld80/s_exp2l.c:38:
/builddir/build/BUILD/openlibm-master/src/types-compat.h:10:0: warning: "__pure2" redefined
 #define __pure2

In file included from ld80/s_exp2l.c:33:0:
/builddir/build/BUILD/openlibm-master/amd64/bsd_cdefs.h:203:0: note: this is the location of the previous definition
 #define __pure2  __attribute__((__const__))

https://kojipkgs.fedoraproject.org//work/tasks/9161/13149161/build.log

nalimilan · 2016-02-27T11:59:30Z

What, the tests pass on PPC64LE? That's news!

Being greedy, I should note that similar warnings appear there:

gcc  -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mcpu=power8 -mtune=power8 -fno-gnu89-inline -std=c99 -Wall -I/builddir/build/BUILD/openlibm-master -I/builddir/build/BUILD/openlibm-master/include -I/builddir/build/BUILD/openlibm-master/ld80 -I/builddir/build/BUILD/openlibm-master/powerpc -I/builddir/build/BUILD/openlibm-master/src -DASSEMBLER -D__BSD_VISIBLE -Wno-implicit-function-declaration -fPIC -c src/s_log1pf.c -o src/s_log1pf.c.o
In file included from src/math_private.h:24:0,
                 from src/s_lrint.c:32,
                 from src/s_llrint.c:9:
src/fpmath.h:47:0: warning: "_LITTLE_ENDIAN" redefined
 #define _LITTLE_ENDIAN    __ORDER_LITTLE_ENDIAN__

<built-in>: note: this is the location of the previous definition
In file included from src/math_private.h:24:0,
                 from src/s_lround.c:33,
                 from src/s_llround.c:11:
src/fpmath.h:47:0: warning: "_LITTLE_ENDIAN" redefined
 #define _LITTLE_ENDIAN    __ORDER_LITTLE_ENDIAN__

<built-in>: note: this is the location of the previous definition
In file included from src/math_private.h:24:0,
                 from src/s_lrint.c:32,
                 from src/s_llrintf.c:9:
src/fpmath.h:47:0: warning: "_LITTLE_ENDIAN" redefined
 #define _LITTLE_ENDIAN    __ORDER_LITTLE_ENDIAN__

<built-in>: note: this is the location of the previous definition

simonbyrne · 2016-02-27T12:12:45Z

What, the tests pass on PPC64LE?

Oh, you missed all the fun yesterday...

simonbyrne · 2016-02-27T12:13:32Z

If you know how to get rid of the redef warnings, I'd be grateful.

nalimilan · 2016-02-27T12:22:38Z

Oh, you missed all the fun yesterday...

Hehe, I was actually going though my e-mail starting from the most recent. Not a the most logical way...

If you know how to get rid of the redef warnings, I'd be grateful.

I hadn't realized it was system-dependent. Actually, that's because my /usr/include/sys/cdefs.h defines _SYS_CDEFS_H, and openlibm checks for _SYS_CDEFS_H_. We should probably check for both, right?

nalimilan · 2016-02-27T12:33:40Z

As regards warnings on PPC, this is because recent gcc versions provide _LITTLE_ENDIAN and _BIG_ENDIAN built-in macros. Looks like we simply need an #ifndef.

ViralBShah · 2016-02-27T14:12:23Z

I have a patch shortly.

nalimilan · 2016-02-27T14:19:54Z

Actually I was also looking into it. We'll be able to compare our changes.

ViralBShah · 2016-02-27T14:21:16Z

This should do it - I think, in bsd_asm.h:

 #ifdef __APPLE__
 #include "../i387/osx_asm.h"
 #define CNAME(x) EXT(x)
 #else
-#include "bsd_cdefs.h"
+//#include "bsd_cdefs.h"
+#include "<sys/cdefs.h>"

nalimilan · 2016-02-27T14:23:15Z

But will that work on all platforms? Why keep bsd_cdefs.h at all then?

ViralBShah · 2016-02-27T14:23:40Z

That's the question I have. If it works on linux and windows, we can get rid of bsd_cdefs.h

nalimilan · 2016-02-27T14:37:26Z

Looks like it works if I get rid of all occurrences of bsd_cdefs.h. How can we test Windows? And would it work on Mac OS X?

nalimilan · 2016-02-27T15:05:29Z

See my nl/includes branch. It gets rid of warnings, but it fails on x86:

make[1]: Entering directory '/builddir/build/BUILD/openlibm-master/test'
gcc  -O2   test-double.c -D__BSD_VISIBLE -I ../include -I../src -L.. -lopenlibm -Wl,-rpath=/builddir/build/BUILD/openlibm-master -o test-double
../libopenlibm.so: undefined reference to `__fpgetprec'
collect2: error: ld returned 1 exit status

https://kojipkgs.fedoraproject.org//work/tasks/9885/13149885/build.log

nalimilan · 2016-02-27T16:38:04Z

Regarding _LITTLE_ENDIAN and _BIG_ENDIAN, the problem is that when gcc defines them, they are boolean flags, while openlibm uses them to store an integer pattern. Could we remove the underscore, or add an OPENLIBM_ prefix? Anyway, these are used in only two or three files.

EDIT: we may as well call them __ORDER_LITTLE_ENDIAN__ and __ORDER_BIG_ENDIAN__ just like gcc.

nalimilan · 2016-02-27T17:20:54Z

BTW, if somebody wants to put a bounty on the first openlibm distribution package supporting PPC... :-) OK, I'm cheating a bit.

ViralBShah · 2016-02-27T18:22:18Z

Either fix is fine on the ENDIAN flags. This should fix the issue I was facing with vs/msun too. I think it is best to add the OPENLIBM_ prefix.

EdSchouten · 2016-02-27T18:31:42Z

2016-02-27 17:38 GMT+01:00 Milan Bouchet-Valat notifications@github.com:

Regarding _LITTLE_ENDIAN and BIG_ENDIAN, the problem is that when gcc defines them, they are boolean flags, while openlibm uses them to store an integer pattern. Could we remove the underscore, or add an OPENLIBM prefix? Anyway, these are used in only two or three files.

I think the preferred way of testing the endianness is to use
BYTE_ORDER and ORDER_LITTLE_ENDIAN/ORDER_BIG_ENDIAN. These
also hold integer patterns:

$ cc -dM -E - < /dev/null | grep ORDER
#define BYTE_ORDER ORDER_LITTLE_ENDIAN
#define ORDER_BIG_ENDIAN 4321
#define ORDER_LITTLE_ENDIAN 1234
#define ORDER_PDP_ENDIAN 3412

Ed Schouten ed@nuxi.nl
Nuxi, 's-Hertogenbosch, the Netherlands
KvK-nr.: 62051717

ViralBShah · 2016-02-27T18:36:54Z

Would be great if you could submit a PR!

nalimilan · 2016-02-28T13:15:00Z

I've made a PR at #125. Indeed, I now think the cleanest solution is to use the gcc/clang macros, and define them when they aren't. That way, the code is easy to read for anybody familiar with these compilers.

ViralBShah · 2016-03-03T09:18:36Z

I think we are good to tag 0.5 now.

simonbyrne · 2016-03-03T09:28:40Z

sounds good to me

nalimilan · 2016-03-03T16:29:29Z

Sorry, looks like I missed another warning with __pure2. I've made a new (smaller) PR: #126

nalimilan · 2016-03-03T16:33:01Z

BTW, there are still a few warnings with GCC6. I've looked at them, and they don't seem to reflect actual bugs. They don't need to be fixed for the release, but it might be good to have a look at some point so that we can spot more easily really problematic ones.

ViralBShah · 2016-03-08T10:35:40Z

Tagged v0.5.

nalimilan · 2016-03-08T11:21:46Z

Could you call it 0.5.0 instead? We'll probably have 0.5.1 after this rather than 0.6.

nalimilan · 2016-03-08T11:27:08Z

Same for openspecfun. I've prepared draft releases.

ViralBShah · 2016-03-08T11:56:12Z

Good point. I may take a few hours to get to it, but if it is easy enough, can you edit it on github?

nalimilan · 2016-03-08T13:18:52Z

Sure, I just did it. You'll have to update your PR JuliaLang/julia#15212 though.

ViralBShah · 2016-03-08T14:01:08Z

Done.

nalimilan mentioned this issue Feb 28, 2016

Fix various warnings #125

Merged

ViralBShah closed this as completed Mar 8, 2016

Tag a new release - 0.5 #117

Tag a new release - 0.5 #117

Comments

ViralBShah commented Feb 23, 2016

tkelman commented Feb 23, 2016

ViralBShah commented Feb 23, 2016

StefanKarpinski commented Feb 23, 2016

tkelman commented Feb 23, 2016

ViralBShah commented Feb 23, 2016

ViralBShah commented Feb 23, 2016

tkelman commented Feb 23, 2016

ViralBShah commented Feb 23, 2016

tkelman commented Feb 23, 2016

ViralBShah commented Feb 23, 2016

ViralBShah commented Feb 23, 2016

tkelman commented Feb 23, 2016

ViralBShah commented Feb 24, 2016

ViralBShah commented Feb 24, 2016

nalimilan commented Feb 24, 2016

ViralBShah commented Feb 24, 2016

tkelman commented Feb 24, 2016

ViralBShah commented Feb 27, 2016

ViralBShah commented Feb 27, 2016

tkelman commented Feb 27, 2016

ViralBShah commented Feb 27, 2016

tkelman commented Feb 27, 2016

ViralBShah commented Feb 27, 2016

tkelman commented Feb 27, 2016

tkelman commented Feb 27, 2016

ViralBShah commented Feb 27, 2016

nalimilan commented Feb 27, 2016

nalimilan commented Feb 27, 2016

nalimilan commented Feb 27, 2016

simonbyrne commented Feb 27, 2016

simonbyrne commented Feb 27, 2016

nalimilan commented Feb 27, 2016

nalimilan commented Feb 27, 2016

ViralBShah commented Feb 27, 2016

nalimilan commented Feb 27, 2016

ViralBShah commented Feb 27, 2016

nalimilan commented Feb 27, 2016

ViralBShah commented Feb 27, 2016

nalimilan commented Feb 27, 2016

nalimilan commented Feb 27, 2016

nalimilan commented Feb 27, 2016

nalimilan commented Feb 27, 2016

ViralBShah commented Feb 27, 2016

EdSchouten commented Feb 27, 2016

ViralBShah commented Feb 27, 2016

nalimilan commented Feb 28, 2016

ViralBShah commented Mar 3, 2016

simonbyrne commented Mar 3, 2016

nalimilan commented Mar 3, 2016

nalimilan commented Mar 3, 2016

ViralBShah commented Mar 8, 2016

nalimilan commented Mar 8, 2016

nalimilan commented Mar 8, 2016

ViralBShah commented Mar 8, 2016

nalimilan commented Mar 8, 2016

ViralBShah commented Mar 8, 2016