Skip to content

Conversation

@massich
Copy link
Owner

@massich massich commented Jan 15, 2018

No description provided.

@massich massich mentioned this pull request Jan 17, 2018
add_executable(CMakeHelloWorld Hello HelloWorld.cpp)

# BLAS libraries compile dynamically against threads
LIST(APPEND CMAKE_THREAD_LIBS_INIT ${LM})
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${LM} is -llibm -llibdl (aka /usr/lib/x86_64-linux-gnu/libm.so and /usr/lib/x86_64-linux-gnu/libdl.so)
This is only needed by gcc compilers, so it should be inside an if statement like if (CMAKE_C_COMPILER MATCHES ".+gcc")

More over it should be discussed if this find_package should be done silently or not.

Advantages of doing it silently are that target_link_libraries(CMakeHelloWorld Hello ${BLAS_LIBRARIES}) is clear. However to make BLA work with static needs to properly compose the thread libs which makes target_link_libraries(CMakeHelloWorld Hello ${BLAS_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}) look like a hack.

Maybe it would be better to just check that Threads is found at the begining of FindBLA.cmake and break if not found, like this is explicit what needs to be done with trheads, and allows for better control. ( In that case, ${LM} should be placed where it's due, and it would make sense Intel10_64lp appending what needs inside FindBLA.cmake)

cc: @agramfort, @aabadie, @kYc0o

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds a good way to clean it up indeed.

@massich
Copy link
Owner Author

massich commented Jan 17, 2018

Just to keep track of it somewhere, this if statement should be reviewed since both branches do the same:

        if (CMAKE_C_COMPILER MATCHES ".+gcc")
          list(APPEND BLAS_SEARCH_LIBS
            # "mkl_intel_lp64 mkl_gnu_thread mkl_core gomp")
            "mkl_intel_lp64 mkl_intel_thread mkl_core iomp5")
        else ()
          list(APPEND BLAS_SEARCH_LIBS
            "mkl_intel_lp64 mkl_intel_thread mkl_core iomp5")
        endif ()

if (CMAKE_C_COMPILER MATCHES ".+gcc")
list(APPEND BLAS_SEARCH_LIBS
"mkl_blas95_lp64 mkl_intel_lp64 mkl_gnu_thread mkl_core iomp5")
# "mkl_blas95_lp64 mkl_intel_lp64 mkl_gnu_thread mkl_core gomp")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I hacked this on mac as there is no gomp on mac os that uses clang without openmp support.

btw we should also test with clang on linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants