[fmatrixtest] Drop dummy.f.

@core Does anyone know, why this was needed? There were no comments in the source about this...

Shouldn't we drop dummy.f all-together? I couldn't find another use of it, but I did not actually test it. The file was added in 2012, probably is was only needed for Autotools

On Sat, Nov 19, 2016 at 04:09:21PM +0000, Christoph Grüninger wrote:

@christi: Whatever your answer was, it was filtered out. Please re-send your comment or edit your old one.

@christi The email-interface strips everything starting from the first quote. You'll have to go TOFU.

@gruenich Dropping dummy.f is what this merge request does. I wrote the title from the point of view of "make fmatrixtest compile". I'll adapt the title to emphasize that were getting rid of dummy.f.

Changed title: [fmatrixtest] Don't compile and link dummy.f. → [fmatrixtest] Drop dummy.f.

I tested your branch, works for me. Unless @markus.blatt has objections, go ahead with merging.

git says that @dominic (is that the right alias? I am getting confused by the non-uniformity of them) added that. So I can't tell why. Just assuming that it might be because of blas or lapack.

Did someone test without blas/lapack installed?

[Edited by @joe: @dominic.kempf -> @dominic]

@gruenich my original mail said, in response to your probably is was only needed for Autotools:

Looking at the logs would reveal that it is some cmake hack.

The logs are not very informative thought!

$ git show 63cb4ee2a5195ab605f17874e3fcad2a367e67e8
commit 63cb4ee2a5195ab605f17874e3fcad2a367e67e8
Author: Markus Blatt <mblatt@dune-project.org>
Date:   Tue Apr 3 08:57:34 2012 +0000

    force linking to fortran libs (needed for static linking)

    [[Imported from SVN: r6601]]

diff --git a/dune/common/test/CMakeLists.txt b/dune/common/test/CMakeLists.txt
index f648242..417b35c 100644
--- a/dune/common/test/CMakeLists.txt
+++ b/dune/common/test/CMakeLists.txt
@@ -22,7 +22,9 @@ add_executable("enumsettest" enumsettest.cc)
 add_executable("fassigntest" fassigntest.cc)
  target_link_libraries("fassigntest" "dunecommon")

-add_executable("fmatrixtest" fmatrixtest.cc)
+# we provide an empty fortran file to force the linker
+# to link to the fortran libraries (needed for static linking)
+add_executable("fmatrixtest" fmatrixtest.cc dummy.f)
 target_link_libraries("fmatrixtest" "dunecommon" ${LAPACK_LIBRARIES})

 add_executable("fvectortest" fvectortest.cc)
 diff --git a/dune/common/test/dummy.f b/dune/common/test/dummy.f
 new file mode 100644
 index 0000000..e69de29

I think it is the wrong fix, as the LAPACK_LIBRARIES should include the fortran libs... Still it seems the file can't be removed at the moment.

@markus.blatt On the system where I noticed that (the one without gfortran), I libblas3 and liblapack3 installed, but no libblas-dev or liblapack-dev. Not sure whether that counts as "not installed".

I also browsed through fmatrixtest.cc before making the branch and could not find anything that looked like one of the usual fortran libraries (in particular no #include or #ifdef).

Yes, ${LAPACK_LIBRARIES} seems to have vanished in the meantime, so I guess dummy.f can really go.

And aren't we always linking statically now? So any problem should have shown up in the builds, but there don't seem to be any.

@markus.blatt I did not add that. My git crawling actually says you added it in 63cb4ee2. I may have changed the line in CMakeLists.txt that changes it, but that was just mindless adapting. I have no idea what it was needed for.

I am sorry for prematurely blaming the wrong person.

The reason might be to force fortran linkage in case a fortran based blas version is used. See https://cmake.org/pipermail/cmake/2011-January/041992.html. It might make sense to add the dummy.f only if a fortran compiler was found.

As a follow-up to the diff that @christi dug up, here is one that removed the linking against lapack:

commit 61efe77b9dbc732d3c985fd0838857bf6c73c1db
Author: Markus Blatt <mblatt@dune-project.org>
Date:   Sun Apr 22 14:14:33 2012 +0000

    Make library dunecommon link to lapack directly. Now lapack does not
    have to specified during linking because it is implied by dunecommon.
    
    [[Imported from SVN: r6644]]

diff --git a/dune/common/test/CMakeLists.txt b/dune/common/test/CMakeLists.txt
index 1c88cd4..4ca5ddb 100644
--- a/dune/common/test/CMakeLists.txt
+++ b/dune/common/test/CMakeLists.txt
@@ -102,8 +102,7 @@ target_link_libraries("float_cmp" "dunecommon")
 # we provide an empty fortran file to force the linker
 # to link to the fortran libraries (needed for static linking)
 add_executable("fmatrixtest" fmatrixtest.cc dummy.f)
-target_link_libraries("fmatrixtest" "dunecommon" ${LAPACK_LIBRARIES})
-
+target_link_libraries("fmatrixtest" "dunecommon")
 add_executable("fvectortest" fvectortest.cc)
 add_executable("gcdlcmtest" gcdlcmtest.cc)
 add_executable("genericiterator_compile_fail" EXCLUDE_FROM_ALL genericiterator_compile_fail.cc)

On another topic, I think I understand now what @markus.blatt means by "static linking": linking against a static liblapack, not building static libs of dune-common and linking those. I will try a static liblapack then, before merging.

Marked this merge request as a Work In Progress

Alright, this is now from a system with gfortran installed. Debian jessie, mostly, and {liblapack-dev, liblapack3} 3.5.0-4 and {libblas-dev, libblas3} 1.2.20110419-10. Linking normally, cmake finds both blas and lapack. Linking statically with the following opts file:

CMAKE_FLAGS="-DCMAKE_CXX_FLAGS=\"-g -O0 -Wall -Wno-literal-suffix\" -DDUNE_USE_ONLY_STATIC_LIBS=ON"

cmake fails to find lapack because:

Determining if the Fortran cheev exists failed with the following output:
Change Dir: /home/joe/Projekte/dune-simd/dune-common/build-cmake/CMakeFiles/CMakeTmp

Run Build Command:"/usr/bin/make" "cmTC_3d7df/fast"
/usr/bin/make -f CMakeFiles/cmTC_3d7df.dir/build.make CMakeFiles/cmTC_3d7df.dir/build
make[1]: Entering directory '/home/joe/Projekte/dune-simd/dune-common/build-cmake/CMakeFiles/CMakeTmp'
Building Fortran object CMakeFiles/cmTC_3d7df.dir/testFortranCompiler.f.o
/usr/bin/gfortran    -c /home/joe/Projekte/dune-simd/dune-common/build-cmake/CMakeFiles/CMakeTmp/testFortranCompiler.f -o CMakeFiles/cmTC_3d7df.dir/testFortranCompiler.f.o
Linking Fortran executable cmTC_3d7df
/usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_3d7df.dir/link.txt --verbose=1
/usr/bin/gfortran       CMakeFiles/cmTC_3d7df.dir/testFortranCompiler.f.o  -o cmTC_3d7df -rdynamic -Wl,--start-group /usr/lib/liblapack.a /usr/lib/libblas.a -Wl,--end-group 
/usr/lib/libblas.a(xerbla.o): In function `xerbla_':
(.text+0x0): multiple definition of `xerbla_'
/usr/lib/liblapack.a(xerbla.o):(.text+0x0): first defined here
collect2: error: ld returned 1 exit status
CMakeFiles/cmTC_3d7df.dir/build.make:99: recipe for target 'cmTC_3d7df' failed
make[1]: *** [cmTC_3d7df] Error 1
make[1]: Leaving directory '/home/joe/Projekte/dune-simd/dune-common/build-cmake/CMakeFiles/CMakeTmp'
Makefile:126: recipe for target 'cmTC_3d7df/fast' failed
make: *** [cmTC_3d7df/fast] Error 2

Seems to me like linking statically against lapack is broken already in the build system. As well as not finding lapack, fvectortest is broken when linking statically:

/usr/lib/ccache/c++   -g -O0 -Wall -Wno-literal-suffix -std=c++14    -pthread CMakeFiles/fvectortest.dir/fvectortest.cc.o  -o fvectortest -rdynamic -Wl,-Bstatic -lgmp -lgmpxx ../../../lib/libdunecommon.a -lblas -Wl,-Bdynamic 
/usr/lib/gcc/x86_64-linux-gnu/4.9/../../../x86_64-linux-gnu/libgmpxx.a(ismpf.o): In function `operator>>(std::istream&, __mpf_struct*)':
(.text+0x126): undefined reference to `__gmpf_set_str'
collect2: error: ld returned 1 exit status
dune/common/test/CMakeFiles/fvectortest.dir/build.make:98: recipe for target 'dune/common/test/fvectortest' failed
make[3]: *** [dune/common/test/fvectortest] Error 1

That however seems to be related to gmpxx.

fmatrixtest however compiles and runs successfully, although it is only linked against -lblas:

/usr/lib/ccache/c++   -g -O0 -Wall -Wno-literal-suffix -std=c++14    -pthread CMakeFiles/fmatrixtest.dir/fmatrixtest.cc.o  -o fmatrixtest -rdynamic -Wl,-Bstatic -lgmp -lgmpxx ../../../lib/libdunecommon.a -lblas -Wl,-Bdynamic 
make[3]: Leaving directory '/home/joe/Projekte/dune-simd/dune-common/build-cmake'
[100%] Built target fmatrixtest

So, since fmatrixtest links against the fortran library -lblas (and because many other tests that never required dummy.f do so, too), I am going to assume that dummy.f is no longer required to link against fortran libraries, not even static ones, and see no reason not to go ahead with the merge.

Linking statically seems to be broken in major ways.

Alright, this is now from a system with gfortran installed. Debian jessie, mostly, and {liblapack-dev, liblapack3} 3.5.0-4 and {libblas-dev, libblas3} 1.2.20110419-10. Linking normally, cmake finds both blas and lapack. Linking statically with the following opts file: [...]

/usr/lib/libblas.a(xerbla.o): In function xerbla_': (.text+0x0): multiple definition of xerbla_' /usr/lib/liblapack.a(xerbla.o):(.text+0x0): first defined here collect2: error: ld returned 1 exit status CMakeFiles/cmTC_3d7df.dir/build.make:99: recipe for target 'cmTC_3d7df' failed

Debian, right? https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=354463 Not our fault is it? We even provided a patch 3 years ago...

So, since fmatrixtest links against the fortran library -lblas (and because many other tests that never required dummy.f do so, too), I am going to assume that dummy.f is no longer required to link against fortran libraries, not even static ones, and see no reason not to go ahead with the merge.

That is a weired kind of reasoning. statically linking is broken anyway on Debian, so we might just as well break it even more on other systems and loose users.

As the linker line is -lbas, you are probably using the dynamic library which carries information that it needs fortran:

ldd /etc/alternatives/libblas.so
    linux-vdso.so.1 (0x00007ffeecf54000)
    libopenblas.so.0 => /usr/lib/libopenblas.so.0 (0x00007efeabc98000)
    libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007efeab997000)
    libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007efeab77a000)
    libgfortran.so.3 => /usr/lib/x86_64-linux-gnu/libgfortran.so.3 (0x00007efeab45c000)
     libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007efeab0b1000)
     /lib64/ld-linux-x86-64.so.2 (0x00007efeadd9d000)
     libquadmath.so.0 => /usr/lib/x86_64-linux-gnu/libquadmath.so.0 (0x00007efeaae74000)
     libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007efeaac5e000)

Please do not break things more than they already are.

Have you tried openblas? Seems less broken to me.

Thanks for the pointer that explains the buildsystem-problem.

And I was linking statically against -lblas, because the linker command line included -Wl,-Bstatic before -lblas, see for example the linker command line for the (failing) fvectortest that I posted earlier:

/usr/lib/ccache/c++   -g -O0 -Wall -Wno-literal-suffix -std=c++14    -pthread CMakeFiles/fvectortest.dir/fvectortest.cc.o  -o fvectortest -rdynamic -Wl,-Bstatic -lgmp -lgmpxx ../../../lib/libdunecommon.a -lblas -Wl,-Bdynamic

So let me now explain my reasoning better: There was the fear that fmatrixtest needs to compile and link dummy.f in order to be able to link against static fortran libraries. I tested this and fmatrixtest still links (and runs successfully) when statically linked to blas, which is a fortran libary, without also linking dummy.f. Therefore the fear is invalid.

Am I missing something?

I do not know. Maybe I am wrong. It seems to work with openblas and atlas.

Unmarked this merge request as a Work In Progress

Mentioned in commit bf4b5fdb

Status changed to merged

[fmatrixtest] Drop dummy.f.

Merge request reports

Activity