Do not hardcode the C++ stdlib in setup.py

rwst commented 6 years ago

sage build with the following flags still uses libstdc++ for some cythonized so files:

export CC="clang"
export CXX="clang++ --stdlib=libc++"
export CXXFLAGS="$CXXFLAGS --stdlib=libc++"
export LDFLAGS="$LDFLAGS -lc++ --stdlib=libc++"

> ldd /home/ralf/sage/local/lib/python2.7/site-packages/sage/libs/ntl/ntl_ZZ.so
    linux-vdso.so.1 (0x00007ffc8f3f1000)
    libntl.so.33 => /home/ralf/sage/local/lib/libntl.so.33 (0x00007f823f6d7000)
    libgmp.so.23 => /home/ralf/sage/local/lib/libgmp.so.23 (0x00007f823f448000)
    libm.so.6 => /lib64/libm.so.6 (0x00007f823f14b000)
    libstdc++.so.6 => /usr/lib64/libstdc++.so.6 (0x00007f823edc2000)
    libpython2.7.so.1.0 => /home/ralf/sage/local/lib/libpython2.7.so.1.0 (0x00007f823e9d8000)
    libpari-gmp-2.10.so.0 => /home/ralf/sage/local/lib/libpari-gmp-2.10.so.0 (0x00007f823def9000)
    libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f823dce2000)
    libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f823dac5000)
    libc.so.6 => /lib64/libc.so.6 (0x00007f823d724000)
    libgf2x.so.1 => /home/ralf/sage/local/lib/libgf2x.so.1 (0x00007f823d501000)
    libc++.so.1 => /usr/lib64/libc++.so.1 (0x00007f823ff05000)
    libc++abi.so.1 => /usr/lib64/libc++abi.so.1 (0x00007f823fec2000)
    /lib64/ld-linux-x86-64.so.2 (0x00007f823fdd6000)
    libdl.so.2 => /lib64/libdl.so.2 (0x00007f823d2fd000)
    libutil.so.1 => /lib64/libutil.so.1 (0x00007f823d0fa000)

CC: @kiwifb @simon-king-jena @jdemeyer

Component: build

Branch/Commit: u/rws/do_not_hardcode_the_c___stdlib_in_setup_py @ 0e4016e

Issue created by migration from https://trac.sagemath.org/ticket/24705

rwst commented 6 years ago

comment:1

Attachment: ntl-10.3.0.log

sagelib is the culprit. It has libstdc++ hardcoded:

../logs/pkgs/sagelib-8.2.beta5.log:[212/474] [211/474] clang++ -pthread -shared -L/home/ralf/sage/local/lib -Wl,-rpath,/home/ralf/sage/local/lib -L/home/ralf/sage/local/lib -L/home/ralf/sage/local/lib -L/home/ralf/sage/local/lib -L/home/ralf/sage/local/lib -L/home/ralf/sage/local/lib -Wl,-rpath,/home/ralf/sage/local/lib -L/home/ralf/sage/local/lib -L/home/ralf/sage/local/lib -L/home/ralf/sage/local/lib -L/home/ralf/sage/local/lib build/temp.linux-x86_64-2.7/build/cythonized/sage/libs/ntl/ntl_ZZ_pX.o -L/home/ralf/sage/local/lib -L/home/ralf/sage/local/lib -lntl -lgmp -lm -lstdc++ -lpython2.7 -o build/lib.linux-x86_64-2.7/sage/libs/ntl/ntl_ZZ_pX.so -lpari

rwst commented 6 years ago

comment:3

This is the relevant part of src/setup.py:

        # Libraries: add stdc++ if needed and sort them
        libs = kwds.get('libraries', [])
        if cplusplus:
            libs = libs + ['stdc++']

rwst commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,4 @@
+sagelib build with --stdlib=libc++ still uses libstdc++ for ntl_ZZ.so

ralf@ark:~/sage/pynac> ldd /home/ralf/sage/local/lib/python2.7/site-packages/sage/libs/ntl/ntl_ZZ.so

rwst commented 6 years ago

comment:4

It seems best to add "--stdlib=libc++"(else the libstdc++ is linked) only if it is already in CFLAGS. See also #24705.

Testing this manually works, except for element_givaro.pyx which still gets compiled without the "--stdlib=libc++" directive.

rwst commented 6 years ago

comment:5

Never mind, I got it compiled, yeah, now testing.

simon-king-jena commented 6 years ago

comment:8

Replying to @rwst:

It seems best to add "--stdlib=libc++"(else the libstdc++ is linked) only if it is already in CFLAGS. See also #24705.

Do you mean #24707?

simon-king-jena commented 6 years ago

comment:10

Replying to @rwst:

This is the relevant part of src/setup.py:

        # Libraries: add stdc++ if needed and sort them
        libs = kwds.get('libraries', [])
        if cplusplus:
            libs = libs + ['stdc++']

Sorry, I don't get it: What would you replace it with, so that the sage library will not use libstdc++ if --stdlib=libc++ is provided?

rwst commented 6 years ago

comment:11

Replying to @simon-king-jena:

Replying to @rwst:
This is the relevant part of src/setup.py:
        # Libraries: add stdc++ if needed and sort them
        libs = kwds.get('libraries', [])
        if cplusplus:
            libs = libs + ['stdc++']
Sorry, I don't get it: What would you replace it with, so that the sage library will not use libstdc++ if --stdlib=libc++ is provided?

At the moment we're still experimenting with flags, see #24707 comment:9. But I already think that the line above needs to be changed to libs = libs + ['c++'] in order to get -lc++ in the link command.

simon-king-jena commented 6 years ago

comment:12

Replying to @rwst:

At the moment we're still experimenting with flags, see #24707 comment:9. But I already think that the line above needs to be changed to libs = libs + ['c++'] in order to get -lc++ in the link command.

But certainly not unconditionally. Or do we want to build with -lc++ when using gcc?

rwst commented 6 years ago

comment:13

Replying to @simon-king-jena:

But certainly not unconditionally. Or do we want to build with -lc++ when using gcc?

Of course (EDIT: not).

rwst commented 6 years ago

Branch: u/rws/do_not_hardcode_the_c___stdlib_in_setup_py

rwst commented 6 years ago

comment:15

This still doesn't catch all ways to smuggle -lstdc++ in the cython file linking stage of sagelib.

New commits:

0e4016e 24705: Do not hardcode the C++ stdlib in setup.py

rwst commented 6 years ago

Commit: 0e4016e

rwst commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,7 +1,14 @@
-sagelib build with --stdlib=libc++ still uses libstdc++ for ntl_ZZ.so
+sage build with the following flags still uses libstdc++ for some cythonized so files:

-ralf@ark:~/sage/pynac> ldd /home/ralf/sage/local/lib/python2.7/site-packages/sage/libs/ntl/ntl_ZZ.so +export CC="clang" +export CXX="clang++ --stdlib=libc++" +export CXXFLAGS="$CXXFLAGS --stdlib=libc++" +export LDFLAGS="$LDFLAGS -lc++ --stdlib=libc++" + + + +> ldd /home/ralf/sage/local/lib/python2.7/site-packages/sage/libs/ntl/ntl_ZZ.so linux-vdso.so.1 (0x00007ffc8f3f1000) libntl.so.33 => /home/ralf/sage/local/lib/libntl.so.33 (0x00007f823f6d7000) libgmp.so.23 => /home/ralf/sage/local/lib/libgmp.so.23 (0x00007f823f448000)

simon-king-jena commented 6 years ago

comment:17

Attachment: sagelib-8.2.beta5.log

I did

export CC="clang"
export CXX="clang++ --stdlib=libc++
export CXXFLAGS="$CXXFLAGS -I/usr/include/libcxxabi/ --stdlib=libc++"
export LDFLAGS="$LDFLAGS -lc++ --stdlib=libc++"
make build

The build process goes pretty far, however it hangs (for an hour or so) in sagelib. See the attached log.

kiwifb commented 6 years ago

comment:18

I am a bit late to this party - sorry. I don't have this problem at all with my clang configured to use libc++ by default. Is it possible to have readelf -d /home/ralf/sage/local/lib/python2.7/site-packages/sage/libs/ntl/ntl_ZZ.so on the object initially posted?

rwst commented 6 years ago

comment:19

So this ticket is wontfix. Use these flags:

export CC="clang"
export CXX="clang++"
export CLANG_DEFAULT_CXX_STDLIB="libc++"

simon-king-jena commented 6 years ago

comment:21

Replying to @rwst:

So this ticket is wontfix. Use these flags:
export CC="clang"
export CXX="clang++"
export CLANG_DEFAULT_CXX_STDLIB="libc++"

Do you claim that this is enough to override what is written in setup.py???

Recall that Sage's setup.py forces usage of libstdc++ and not libc++.

simon-king-jena commented 6 years ago

comment:22

Replying to @simon-king-jena:

Do you claim that this is enough to override what is written in setup.py???

Please let me first test whether the given flags are enough to let the branch from #24710 build on ubuntu. Or do you even claim that with your flags scipy will build without modifications?

kiwifb commented 6 years ago

comment:23

setup.py adds -lstdc++ to the flags used for linking but that can still be discarded by the compiler driver used. I can build sage with clang without altering setup.py so you should be able to. I am glad that Ralph found that environment variable.

rwst commented 6 years ago

comment:24

Replying to @simon-king-jena:

Replying to @simon-king-jena:

Do you claim that this is enough to override what is written in setup.py???

Please let me first test whether the given flags are enough to let the branch from #24710 build on ubuntu. Or do you even claim that with your flags scipy will build without modifications?

I do not make such claims without testing first.

simon-king-jena commented 6 years ago

Attachment: giac-1.4.9.45.p1.log

simon-king-jena commented 6 years ago

comment:25

I just tried with the branch from #24710 and with the flags stated in comment:19. But giac fails to build, see attachment: giac-1.4.9.45.p1.log

kiwifb commented 6 years ago

comment:26

Replying to @simon-king-jena:

I just tried with the branch from #24710 and with the flags stated in comment:19. But giac fails to build, see attachment: giac-1.4.9.45.p1.log

It looks like you need #24696

simon-king-jena commented 6 years ago

comment:27

Replying to @kiwifb:

Replying to @simon-king-jena:

I just tried with the branch from #24710 and with the flags stated in comment:19. But giac fails to build, see attachment: giac-1.4.9.45.p1.log

It looks like you need #24696

OK, trying again with #24710 and #24696

simon-king-jena commented 6 years ago

comment:28

Hooray, it builds now! I didn't run tests though. Anyway, to summarize what I did on ubuntu:

install clang, clang-dev, libc++abi-dev
checkout/merge #24710 and #24696

export CC="clang"
export CXX="clang++"
export CLANG_DEFAULT_CXX_STDLIB="libc++"

make

So, your finding out about CLANG_DEFAULT_CXX_STDLIB was great!

Side-remarks:

I did not have the impression that building Sage with clang was faster than with gcc.
I will now do some small timings to see about the performance of the code generated by clang vs. gcc.

simon-king-jena commented 6 years ago

comment:29

I just realise that using #24710 was supposed to be not needed (it was closed as invalid).

Anyway, a data point concerning timings, after installing meataxe:

With gcc

sage: M = random_matrix(GF(9,'x'), 5000)
sage: type(M)
<type 'sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense'>
sage: N = random_matrix(GF(9,'x'), 5000)
sage: %timeit _=M-N
10 loops, best of 3: 20.5 ms per loop
sage: %timeit _=M*N
1 loop, best of 3: 46.5 s per loop

With clang:

sage: M = random_matrix(GF(9,'x'), 5000)
sage: type(M)
<type 'sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense'>
sage: N = random_matrix(GF(9,'x'), 5000)
sage: %timeit _=M-N
10 loops, best of 3: 20.5 ms per loop
sage: %timeit _=M*N
1 loop, best of 3: 53.2 s per loop

Since meataxe is heavily used in the computations that I really care about (group cohomology), I am not so happy about a slow-down of almost 15%. So, probably I won't use clang in my computations. Nonetheless, it is good for portability that clang can also be used on ubuntu.

simon-king-jena commented 6 years ago

comment:30

I get several segfaults when testing sage.homology. Is that to be expected?

jdemeyer commented 6 years ago

comment:31

Should this ticket be closed?

simon-king-jena commented 6 years ago

comment:32

Replying to @jdemeyer:

Should this ticket be closed?

I think so. Certainly the test failure should be for a different ticket.

sagemath / sage

Do not hardcode the C++ stdlib in setup.py #24705