Skip to content

Fix LAPACK definitions #2239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 16, 2018
Merged

Fix LAPACK definitions #2239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c8f1f9
Fix LAPACK functions with MKL
umar456 Jul 15, 2018
7a3a5b8
Remove reference to INTEL_MKL_ROOT_DIR env variable. Use MKLROOT
umar456 Jul 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 9 additions & 14 deletions CMakeModules/FindLAPACKE.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,19 @@
SET(LAPACKE_ROOT_DIR CACHE STRING
"Root directory for custom LAPACK implementation")

IF (NOT INTEL_MKL_ROOT_DIR)
SET(INTEL_MKL_ROOT_DIR $ENV{INTEL_MKL_ROOT})
ENDIF()

IF(NOT LAPACKE_ROOT_DIR)

IF (ENV{LAPACKEDIR})
if(NOT LAPACKE_ROOT_DIR)
if (ENV{LAPACKEDIR})
SET(LAPACKE_ROOT_DIR $ENV{LAPACKEDIR})
ENDIF()
endif()

IF (ENV{LAPACKE_ROOT_DIR})
if (ENV{LAPACKE_ROOT_DIR})
SET(LAPACKE_ROOT_DIR $ENV{LAPACKE_ROOT_DIR})
ENDIF()
endif()

IF (INTEL_MKL_ROOT_DIR)
SET(LAPACKE_ROOT_DIR ${INTEL_MKL_ROOT_DIR})
ENDIF()
ENDIF()
if (ENV{MKLROOT})
SET(LAPACKE_ROOT_DIR $ENV{MKLROOT})
endif()
endif()

# Check if we can use PkgConfig
FIND_PACKAGE(PkgConfig)
Expand Down
23 changes: 4 additions & 19 deletions src/backend/common/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ target_sources(afcommon_interface
${CMAKE_CURRENT_SOURCE_DIR}/err_common.hpp
${CMAKE_CURRENT_SOURCE_DIR}/host_memory.cpp
${CMAKE_CURRENT_SOURCE_DIR}/host_memory.hpp
${CMAKE_CURRENT_SOURCE_DIR}/lapacke.cpp
${CMAKE_CURRENT_SOURCE_DIR}/lapacke.hpp
Copy link
Member

Choose a reason for hiding this comment

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

If LAPACK libs or MKL was not found, then wouldn't this cause an issue ? We can build without linear algebra support earlier, wouldn't this break that ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Those files have ifdefs for apple/OSX. Accelerate framework is always installed on that system. On Linux and windows we don't need those files but the hpp file is included in a couple of places.

${CMAKE_CURRENT_SOURCE_DIR}/module_loading.hpp
${CMAKE_CURRENT_SOURCE_DIR}/sparse_helpers.hpp
${CMAKE_CURRENT_SOURCE_DIR}/util.cpp
Expand All @@ -40,9 +42,9 @@ target_sources(afcommon_interface
)

if(WIN32)
target_sources(afcommon_interface INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/module_loading_windows.cpp)
target_sources(afcommon_interface INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/module_loading_windows.cpp)
else()
target_sources(afcommon_interface INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/module_loading_unix.cpp)
target_sources(afcommon_interface INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/module_loading_unix.cpp)
endif()

target_include_directories(afcommon_interface
Expand All @@ -51,9 +53,6 @@ target_include_directories(afcommon_interface
${PROJECT_BINARY_DIR}
)

add_library(afcommon_lapack_interface INTERFACE)


if(AF_WITH_LOGGING)
dependency_check(spdlog_FOUND "spdlog not found.")
target_compile_definitions(afcommon_interface
Expand Down Expand Up @@ -111,17 +110,3 @@ if(AF_WITH_GRAPHICS)
add_dependencies(afcommon_interface forge-ext)
endif()
endif()

if(LAPACK_FOUND)
target_sources(afcommon_lapack_interface
INTERFACE
${CMAKE_CURRENT_SOURCE_DIR}/lapacke.cpp
${CMAKE_CURRENT_SOURCE_DIR}/lapacke.hpp
)

target_include_directories(afcommon_lapack_interface
INTERFACE ${LAPACK_INCLUDE_DIR})

target_compile_definitions(afcommon_lapack_interface
INTERFACE WITH_LINEAR_ALGEBRA)
endif()
17 changes: 14 additions & 3 deletions src/backend/cpu/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,6 @@ if(USE_CPU_MKL)
c_api_interface
cpp_api_interface
afcommon_interface
afcommon_lapack_interface
cpu_sort_by_key
MKL::MKL
Threads::Threads
Expand All @@ -336,14 +335,26 @@ else()
c_api_interface
cpp_api_interface
afcommon_interface
afcommon_lapack_interface
cpu_sort_by_key
${CBLAS_LIBRARIES}
${LAPACK_LIBRARIES}
FFTW::FFTW
FFTW::FFTWF
Threads::Threads
)
if(LAPACK_FOUND)
target_link_libraries(afcpu
PRIVATE
${LAPACK_LIBRARIES})
target_include_directories(afcpu
PRIVATE
${LAPACK_INCLUDE_DIR})
endif()
endif()

if(LAPACK_FOUND OR MKL_FOUND)
target_compile_definitions(afcpu
PRIVATE
WITH_LINEAR_ALGEBRA)
endif()

install(TARGETS afcpu
Expand Down
21 changes: 7 additions & 14 deletions src/backend/opencl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ if(AF_WITH_GRAPHICS)
)
endif()

if(LAPACK_FOUND)
if(LAPACK_FOUND OR MKL_FOUND)
target_sources(afopencl
PRIVATE
magma/gebrd.cpp
Expand Down Expand Up @@ -486,18 +486,15 @@ if(LAPACK_FOUND)
magma/ungqr.cpp
magma/unmqr.cpp
#magma/unmqr2.cpp

)
)

if(USE_OPENCL_MKL)
dependency_check(MKL_FOUND "MKL not found")
target_compile_definitions(afopencl PRIVATE USE_MKL)

target_link_libraries(afopencl
PRIVATE
MKL::MKL
)

MKL::MKL)
else()
dependency_check(OpenCL_FOUND "OpenCL not found.")

Expand All @@ -508,7 +505,8 @@ if(LAPACK_FOUND)
dependency_check(CBLAS_LIBRARIES "CBLAS not found.")
target_include_directories(afopencl
PRIVATE
${CBLAS_INCLUDE_DIR})
${CBLAS_INCLUDE_DIR}
${LAPACK_INCLUDE_DIR})
target_link_libraries(afopencl
PRIVATE
${CBLAS_LIBRARIES}
Copy link
Member

Choose a reason for hiding this comment

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

cpu cmake file adds LAPACK_LIBRARIES. Isn't it needed here as well ?

Copy link
Member

Choose a reason for hiding this comment

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

nvm, I couldn't see the line after CBLAS_LIBRARIES in the diff.

Expand All @@ -518,13 +516,8 @@ if(LAPACK_FOUND)
target_compile_definitions(
afopencl
PRIVATE
WITH_OPENCL_LINEAR_ALGEBRA
)

target_link_libraries(afopencl
PRIVATE
afcommon_lapack_interface)
endif(LAPACK_FOUND)
WITH_LINEAR_ALGEBRA)
endif(LAPACK_FOUND OR MKL_FOUND)

install(TARGETS afopencl
EXPORT ArrayFireOpenCLTargets
Expand Down