Skip to content

CMake: Rewrite FindMKL.cmake - #7595

Merged
mohanchen merged 8 commits into
deepmodeling:developfrom
Growl1234:cmake-mkl
Jul 10, 2026
Merged

mohanchen merged 8 commits into
deepmodeling:developfrom
Growl1234:cmake-mkl

Conversation

@Growl1234

@Growl1234 Growl1234 commented Jul 6, 2026 •

Copy link
Copy Markdown

This reworks FindMKL.cmake to model the oneMKL link closure required by ABACUS rather than exposing individual MKL libraries to callers.

ABACUS directly uses LP64 Fortran-style BLAS/LAPACK symbols and, in MPI builds, BLACS/ScaLAPACK routines. The finder now provides:

  • abacus::mkl for the BLAS/LAPACK and FFTW compatibility interfaces;
  • abacus::mkl_scalapack for the MPI closure, including ScaLAPACK, the matching BLACS library, base MKL libraries, and MPI::MPI_CXX.

The configuration handles the relevant oneMKL choices explicitly:

  • LP64 only, since ABACUS integer arguments are not ILP64-compatible;
  • mkl_gf_lp64 for GCC and mkl_intel_lp64 otherwise, preserving the established ABACUS interface-library selection;
  • Sequential, GNU-threaded, or Intel-threaded MKL;
  • Static or shared oneMKL libraries, with a local linker group for static archive closures on Linux;
  • Open MPI through mkl_blacs_openmpi_lp64, while MPICH and Intel MPI use mkl_blacs_intelmpi_lp64, as required by oneMKL.

Closes #7568.

Note about visible affects

  1. For MKL_MPI=AUTO, MPI library-version probing is enabled before find_package(MPI) when the build is not cross-compiled. This allows Open MPI to be detected reliably, but does not work for cross builds. People who process cross builds must select the BLACS interface explicitly through MKL_MPI when using MKL.
  2. If you use MKL with OpenMPI, better be careful of the status message -- oneMKL BLACS interface: ${_mkl_blacs_name} and ensure ${_mkl_blacs_name} here is mkl_blacs_openmpi_lp64; normally this should work fine.
  3. Tested compiler sets: GNU, IntelLLVM, LLVM. Other compilers might need care because only GNU uses mkl_gf_lp64.
  4. This PR targets tested Linux configurations only (including Windows WSL2) as ABACUS is a Linux-based program anyways.
  5. Original cache variables such as MKL_INCLUDE are no longer available to pass manually, as this is the responsibility of internal logic.

Copilot AI review requested due to automatic review settings July 6, 2026 15:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request rewrites cmake/modules/FindMKL.cmake to expose ABACUS-oriented oneMKL link closures (BLAS/LAPACK/FFTW, plus BLACS/ScaLAPACK+MPI for MPI builds) instead of exposing individual MKL libraries, and adjusts the build to support MPI-flavor auto-detection for the BLACS interface.

Changes:

  • Replace FindMKL.cmake with a closure-based design that provides MKL::MKL and MKL::MKL_SCALAPACK, plus cache axes for link mode/threading/MPI ABI.
  • Enable MPI library-version probing (when not cross-compiling) so MKL can auto-select the correct BLACS interface.
  • Switch the core linalg link closure to consume ${MKL_LIBRARIES} and key off MKL_FOUND.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
cmake/modules/FindMKL.cmake Reworked MKL discovery to build ABACUS-specific imported-target closures with explicit threading/link-mode/MPI-ABI handling.
CMakeLists.txt Enables MPI_DETERMINE_LIBRARY_VERSION pre-probing for improved Open MPI detection used by the MKL finder.
source/CMakeLists.txt Updates the linalg closure to link via ${MKL_LIBRARIES} and key off MKL_FOUND.

Comment thread cmake/modules/FindMKL.cmake Outdated
Comment thread cmake/modules/FindMKL.cmake
Comment thread cmake/modules/FindMKL.cmake
Comment thread cmake/modules/FindMKL.cmake Outdated
Comment thread source/CMakeLists.txt
@Growl1234
Growl1234 force-pushed the cmake-mkl branch 3 times, most recently from ecf17f8 to c92fe8b Compare July 8, 2026 04:41
@mohanchen
mohanchen requested a review from QuantumMisaka July 8, 2026 09:39

@QuantumMisaka QuantumMisaka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I expanded the MKL-path review across the documented build/install flows. The existing Docker, toolchain, and conda docs mostly still drive MKL through MKLROOT, so those paths explain why CI stays green. The new MKL_ROOT entry point added by this PR is still not propagated consistently.

Comment thread CMakeLists.txt Outdated
Comment thread cmake/CollectBuildInfoVars.cmake
@mohanchen mohanchen added the Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS label Jul 8, 2026

@QuantumMisaka QuantumMisaka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@Growl1234
Growl1234 force-pushed the cmake-mkl branch 2 times, most recently from 4dabab4 to 7a8c468 Compare July 10, 2026 04:32
@mohanchen
mohanchen merged commit 52a15fc into deepmodeling:develop Jul 10, 2026
17 checks passed
@Growl1234
Growl1234 deleted the cmake-mkl branch July 24, 2026 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code scan] Require MKL interface library in non-MPI detection

5 participants