diff --git a/backends/mlx/CMakeLists.txt b/backends/mlx/CMakeLists.txt index acb96fb1ed9..15db7ca0cb0 100644 --- a/backends/mlx/CMakeLists.txt +++ b/backends/mlx/CMakeLists.txt @@ -162,83 +162,78 @@ if(NOT _mlx_deployment_target_ok) ) endif() -# MLX build options - we only need the C++ library with Metal -set(MLX_BUILD_PYTHON_BINDINGS - OFF - CACHE BOOL "" FORCE +# Build MLX in its own isolated CMake scope via ExternalProject, then consume it +# as a prebuilt static lib through an imported target (see below). Building MLX +# with add_subdirectory would drop its whole project into ExecuTorch's +# target/option namespace, which collides with shared deps MLX fetches (e.g. +# nlohmann_json) and leaks MLX's MLX_BUILD_* options into our cache. The +# isolated scope runs MLX's FetchContent in its own namespace, so no collision +# and no submodule patching are needed. +include(ExternalProject) + +set(_mlx_binary_dir ${CMAKE_CURRENT_BINARY_DIR}/mlx) +set(_mlx_static_lib ${_mlx_binary_dir}/libmlx.a) +# With MLX_METAL_JIT=ON, MLX does not install the metallib; it is produced in +# the build tree at this path. +set(_mlx_metallib ${_mlx_binary_dir}/mlx/backend/metal/kernels/mlx.metallib) + +message( + STATUS "Building MLX from submodule (ExternalProject): ${MLX_SOURCE_DIR}" ) -set(MLX_BUILD_TESTS - OFF - CACHE BOOL "" FORCE +ExternalProject_Add( + mlx_external + SOURCE_DIR ${MLX_SOURCE_DIR} + BINARY_DIR ${_mlx_binary_dir} + CMAKE_ARGS -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} + -DCMAKE_CXX_STANDARD=${CMAKE_CXX_STANDARD} + -DCMAKE_OSX_DEPLOYMENT_TARGET=${CMAKE_OSX_DEPLOYMENT_TARGET} + -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE} + -DPLATFORM=${PLATFORM} + -DDEPLOYMENT_TARGET=${DEPLOYMENT_TARGET} + # We only need the static Metal C++ library. + -DMLX_BUILD_METAL=ON + -DMLX_BUILD_CPU=OFF + -DMLX_BUILD_CUDA=OFF + -DMLX_BUILD_SHARED_LIBS=OFF + -DMLX_BUILD_PYTHON_BINDINGS=OFF + -DMLX_BUILD_PYTHON_STUBS=OFF + -DMLX_BUILD_TESTS=OFF + -DMLX_BUILD_EXAMPLES=OFF + -DMLX_BUILD_BENCHMARKS=OFF + -DMLX_BUILD_GGUF=OFF + -DMLX_BUILD_SAFETENSORS=OFF + -DMLX_METAL_JIT=ON + # MLX's own install() does not emit libmlx.a where we consume it or the + # metallib at all, so skip the install step and read both from the build tree. + INSTALL_COMMAND "" + # ExternalProject stamps its build, so a bare MLX submodule bump (git + # submodule update) would not invalidate the stamp and we'd link a stale + # libmlx.a with no signal. BUILD_ALWAYS reruns the build step every configure; + # it is a fast no-op under Ninja when nothing changed. + BUILD_ALWAYS ON + # Required so Ninja knows these are produced by the external build. + BUILD_BYPRODUCTS ${_mlx_static_lib} ${_mlx_metallib} ) -set(MLX_BUILD_EXAMPLES - OFF - CACHE BOOL "" FORCE -) -set(MLX_BUILD_BENCHMARKS - OFF - CACHE BOOL "" FORCE -) -set(MLX_BUILD_PYTHON_STUBS - OFF - CACHE BOOL "" FORCE -) -set(MLX_BUILD_CUDA - OFF - CACHE BOOL "" FORCE -) -set(MLX_BUILD_CPU - OFF - CACHE BOOL "" FORCE -) -set(MLX_BUILD_METAL - ON - CACHE BOOL "" FORCE -) -set(MLX_BUILD_SHARED_LIBS - OFF - CACHE BOOL "" FORCE -) -set(MLX_BUILD_GGUF - OFF - CACHE BOOL "" FORCE -) -set(MLX_BUILD_SAFETENSORS - OFF - CACHE BOOL "" FORCE -) -set(MLX_METAL_JIT - ON - CACHE BOOL "Use JIT compiled Metal kernels" -) - -# Auto-apply patches to MLX submodule. Each patch is applied idempotently: `git -# apply --check` tests whether the patch is still applicable (i.e. not yet -# applied), and only then applies it. -set(_mlx_patches "${CMAKE_CURRENT_SOURCE_DIR}/patches/mlx_json.patch") -foreach(_patch IN LISTS _mlx_patches) - if(EXISTS "${_patch}" AND EXISTS "${MLX_SOURCE_DIR}") - get_filename_component(_patch_name "${_patch}" NAME) - execute_process( - COMMAND git apply --check "${_patch}" - WORKING_DIRECTORY ${MLX_SOURCE_DIR} - RESULT_VARIABLE _patch_check - OUTPUT_QUIET ERROR_QUIET - ) - if(_patch_check EQUAL 0) - execute_process( - COMMAND git apply "${_patch}" WORKING_DIRECTORY ${MLX_SOURCE_DIR} - ) - message(STATUS "Applied ${_patch_name} to MLX submodule") - else() - message(STATUS "${_patch_name} already applied or not applicable") - endif() - endif() -endforeach() -# Add MLX subdirectory -message(STATUS "Adding MLX from submodule: ${MLX_SOURCE_DIR}") -add_subdirectory(${MLX_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR}/mlx) +# Imported target for the MLX static library produced by mlx_external. A static +# libmlx.a carries no transitive link deps, so re-add the frameworks MLX itself +# links (mirrors third-party/mlx/CMakeLists.txt:209). CPU is OFF, so Accelerate +# is not needed. +add_library(mlx STATIC IMPORTED GLOBAL) +set_target_properties(mlx PROPERTIES IMPORTED_LOCATION ${_mlx_static_lib}) +# Headers come from the submodule source tree (always present), not the build +# dir, so mlxdelegate compilation cannot race the external build. Verified: MLX +# emits no public headers into the build tree (version.h is checked in; the +# version string is a compile-def on version.cpp), so the source dir suffices. +target_include_directories(mlx INTERFACE ${MLX_SOURCE_DIR}) +find_library(METAL_FRAMEWORK Metal) +find_library(FOUNDATION_FRAMEWORK Foundation) +find_library(QUARTZ_FRAMEWORK QuartzCore) +set_target_properties( + mlx + PROPERTIES INTERFACE_LINK_LIBRARIES + "${METAL_FRAMEWORK};${FOUNDATION_FRAMEWORK};${QUARTZ_FRAMEWORK}" +) # ----------------------------------------------------------------------------- # MLX Backend library @@ -266,6 +261,10 @@ add_library(mlxdelegate ${_mlx_backend__srcs}) # Ensure schema is generated before compiling add_dependencies(mlxdelegate mlx_schema) +# mlx is an imported target, so a dependency on it would not order the build. +# Depend on mlx_external directly so libmlx.a exists before mlxdelegate links. +add_dependencies(mlxdelegate mlx_external) + # Add logging flag if enabled if(ET_MLX_ENABLE_OP_LOGGING) target_compile_definitions(mlxdelegate PRIVATE ET_MLX_ENABLE_OP_LOGGING=1) @@ -283,8 +282,9 @@ target_include_directories( mlxdelegate PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/runtime ) -# Link against MLX and executorch mlx is only available at BUILD_INTERFACE - -# consumers must link to mlx separately +# Link against MLX and executorch. mlx is an imported static lib (built by +# mlx_external); it is only consumed at build time here, not re-exported, so +# downstream consumers must link to mlx separately via the package config. target_link_libraries( mlxdelegate PRIVATE mlx_schema executorch_core $ ) @@ -301,8 +301,10 @@ install( DESTINATION ${CMAKE_INSTALL_LIBDIR} ) -# Install mlx library for downstream consumers -install(TARGETS mlx DESTINATION ${CMAKE_INSTALL_LIBDIR}) +# Install mlx library for downstream consumers. mlx is an imported target (built +# by mlx_external), so install the static lib file directly rather than via +# install(TARGETS ...), which only accepts built targets. +install(FILES ${_mlx_static_lib} DESTINATION ${CMAKE_INSTALL_LIBDIR}) # Install mlx headers for downstream consumers that may need mlx types install( @@ -325,16 +327,15 @@ install( # containing MLX code. When MLX is statically linked into _portable_lib.so, this # is the directory containing _portable_lib.so. # -# For the installed library, we put metallib in lib/ alongside libmlx.a -install( - FILES ${CMAKE_CURRENT_BINARY_DIR}/mlx/mlx/backend/metal/kernels/mlx.metallib - DESTINATION ${CMAKE_INSTALL_LIBDIR} -) +# For the installed library, we put metallib in lib/ alongside libmlx.a. The +# metallib is produced in the mlx_external build tree (MLX_METAL_JIT=ON does not +# install it); _mlx_metallib points there. +install(FILES ${_mlx_metallib} DESTINATION ${CMAKE_INSTALL_LIBDIR}) # Cache the metallib path for pybindings to copy it next to _portable_lib.so # This enables editable installs to work correctly set(MLX_METALLIB_PATH - "${CMAKE_CURRENT_BINARY_DIR}/mlx/mlx/backend/metal/kernels/mlx.metallib" + "${_mlx_metallib}" CACHE INTERNAL "Path to mlx.metallib for pybindings" ) diff --git a/backends/mlx/patches/mlx_json.patch b/backends/mlx/patches/mlx_json.patch deleted file mode 100644 index 4760403c8e6..00000000000 --- a/backends/mlx/patches/mlx_json.patch +++ /dev/null @@ -1,29 +0,0 @@ -diff --git a/CMakeLists.txt b/CMakeLists.txt ---- a/CMakeLists.txt -+++ b/CMakeLists.txt -@@ -304,12 +304,18 @@ else() - set(MLX_BUILD_ACCELERATE OFF) - endif() - --message(STATUS "Downloading json") --FetchContent_Declare( -- json -- URL https://github.com/nlohmann/json/releases/download/v3.11.3/json.tar.xz) --FetchContent_MakeAvailable(json) --target_include_directories( -- mlx PRIVATE $) -+# Only fetch json if nlohmann_json target doesn't already exist -+# (ExecuTorch provides its own copy) -+if(NOT TARGET nlohmann_json) -+ message(STATUS "Downloading json") -+ FetchContent_Declare( -+ json -+ URL https://github.com/nlohmann/json/releases/download/v3.11.3/json.tar.xz) -+ FetchContent_MakeAvailable(json) -+ target_include_directories( -+ mlx PRIVATE $) -+else() -+ message(STATUS "Using existing nlohmann_json target") -+endif() - - add_subdirectory(${CMAKE_CURRENT_LIST_DIR}/mlx) diff --git a/tools/cmake/executorch-config.cmake b/tools/cmake/executorch-config.cmake index 524e1be36ec..946436e3083 100644 --- a/tools/cmake/executorch-config.cmake +++ b/tools/cmake/executorch-config.cmake @@ -134,14 +134,23 @@ if(TARGET mlxdelegate) if(_mlx_library) add_library(mlx STATIC IMPORTED) set_target_properties(mlx PROPERTIES IMPORTED_LOCATION "${_mlx_library}") - # MLX requires Metal and Foundation frameworks on Apple platforms + # libmlx.a is a static archive with no transitive link deps, so re-add the + # frameworks MLX links PUBLIC (mirrors + # third-party/mlx/CMakeLists.txt:209). Must match the in-tree imported + # target in backends/mlx/CMakeLists.txt. if(APPLE) find_library(METAL_FRAMEWORK Metal) find_library(FOUNDATION_FRAMEWORK Foundation) - if(METAL_FRAMEWORK AND FOUNDATION_FRAMEWORK) + find_library(QUARTZ_FRAMEWORK QuartzCore) + if(METAL_FRAMEWORK + AND FOUNDATION_FRAMEWORK + AND QUARTZ_FRAMEWORK + ) set_target_properties( - mlx PROPERTIES INTERFACE_LINK_LIBRARIES - "${METAL_FRAMEWORK};${FOUNDATION_FRAMEWORK}" + mlx + PROPERTIES + INTERFACE_LINK_LIBRARIES + "${METAL_FRAMEWORK};${FOUNDATION_FRAMEWORK};${QUARTZ_FRAMEWORK}" ) endif() endif()