diff --git a/.agents/state.md b/.agents/state.md index 4c3604f4..1af0b9e5 100644 --- a/.agents/state.md +++ b/.agents/state.md @@ -34536,3 +34536,18 @@ unchanged. A RED-first source regression test pins that ownership boundary. This is build portability only and changes no runtime, model lifecycle, correctness result, or benchmark disposition. Darwin consumer CI remains the binding AppleClang verification. + +## 2026-08-03 - MLX system-header dependency boundary + +LocalAI Darwin consumer run `30783379823` showed that translation-unit warning +flags and pragmas did not durably isolate AppleClang diagnostics emitted by MLX +0.29.3 headers from vllm.cpp's target-wide `-Werror`. The MLX library and its +public includes are now owned by a dedicated imported CMake target, with the +include directory explicitly present in both the normal and SYSTEM interface. +The provider links that dependency target and carries no diagnostic pragma. + +A RED-first focused regression configures a synthetic MLX dependency whose +header emits Clang's GNU-folding warning: the dependency warning must compile, +while the identical warning in project source must remain fatal. This is build +portability only. Local structural checks pass; Darwin consumer CI remains the +binding AppleClang verification. diff --git a/CMakeLists.txt b/CMakeLists.txt index 2cbd6d4c..b3643363 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -831,11 +831,13 @@ if(VLLM_CPP_METAL) message(FATAL_ERROR "VLLM_CPP_MLX=ON but no libmlx + include/mlx/array.h under " "MLX_ROOT=${MLX_ROOT}") endif() + include(cmake/MLXDependency.cmake) + vllm_cpp_import_mlx("${MLX_ROOT}" "${MLX_LIBRARY}") target_compile_definitions(vllm PUBLIC VLLM_CPP_MLX) target_sources(vllm PRIVATE src/vt/metal/metal_mlx_provider.mm) - # SYSTEM so MLX's own headers cannot break our -Werror build. - target_include_directories(vllm SYSTEM PRIVATE "${MLX_ROOT}/include") - target_link_libraries(vllm PUBLIC ${MLX_LIBRARY}) + # MLX is a third-party dependency target: its public headers are SYSTEM, + # while vllm's own sources remain subject to the target-wide -Werror policy. + target_link_libraries(vllm PUBLIC vllm_cpp::mlx) message(STATUS "MLX GEMM provider enabled: ${MLX_LIBRARY}") endif() endif() diff --git a/cmake/MLXDependency.cmake b/cmake/MLXDependency.cmake new file mode 100644 index 00000000..a33a86f1 --- /dev/null +++ b/cmake/MLXDependency.cmake @@ -0,0 +1,12 @@ +function(vllm_cpp_import_mlx mlx_root mlx_library) + if(TARGET vllm_cpp::mlx) + return() + endif() + + add_library(vllm_cpp_mlx UNKNOWN IMPORTED) + set_target_properties(vllm_cpp_mlx PROPERTIES + IMPORTED_LOCATION "${mlx_library}" + INTERFACE_INCLUDE_DIRECTORIES "${mlx_root}/include" + INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${mlx_root}/include") + add_library(vllm_cpp::mlx ALIAS vllm_cpp_mlx) +endfunction() diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md index 2b0965c0..517b45f6 100644 --- a/docs/BENCHMARKS.md +++ b/docs/BENCHMARKS.md @@ -6783,10 +6783,11 @@ us by about 1.5 points. The default (non-MLX) build is **95.9%** against this corrected baseline, not 96.4%. -The 2026-08-02 AppleClang warning-suppression correction is **NOT APPLICABLE** -to benchmark results: it changes only whether the optional MLX provider compiles -under target-wide `-Werror`. Darwin CI remains the build-verification gate; no -runtime path, measurement, or binding number changed. +The 2026-08-03 MLX system-header dependency correction is **NOT APPLICABLE** to +benchmark results: it replaces translation-unit warning suppression with an +imported CMake dependency whose public headers are `SYSTEM`. Darwin CI remains +the build-verification gate; no runtime path, measurement, or binding number +changed. **Everything qualitative in the entry below still holds** — MLX wins prefill, the shape gate is the right disposition, the fallback hoist was worth 27.2 vs 17.8 — diff --git a/docs/STATUS.md b/docs/STATUS.md index f2225a2b..c23e69d9 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -1435,9 +1435,10 @@ loop, not parallelism or layout. An optional MLX GEMM provider is available via `-DVLLM_CPP_MLX=ON` and currently measures net slower than our own kernels. Full per-lever chronology: [docs/BENCHMARKS.md](BENCHMARKS.md) and [.agents/specs/metal-dispatch-attribution.md](../.agents/specs/metal-dispatch-attribution.md). -The MLX-enabled Darwin build remains **build-verification pending**: its narrow -AppleClang header-warning suppression now lives in the provider translation unit -so the target-wide `-Werror` cannot re-promote it; Darwin CI is the binding gate. +The MLX-enabled Darwin build remains **build-verification pending**: MLX is now +an imported CMake dependency whose public include directory is explicitly +`SYSTEM`; third-party header diagnostics stay outside project `-Werror`, while +vllm.cpp warnings remain fatal. Darwin CI is the binding AppleClang gate. **CUDA architectures.** The production target is GB10/`sm_121a` (runtime-gated, both gate models token-exact + at/above vLLM speed). The arch-additivity diff --git a/src/vt/metal/metal_mlx_provider.mm b/src/vt/metal/metal_mlx_provider.mm index e85a5aa6..524fbb7c 100644 --- a/src/vt/metal/metal_mlx_provider.mm +++ b/src/vt/metal/metal_mlx_provider.mm @@ -50,13 +50,6 @@ #include #include -// MLX 0.29.x uses constant-folded variable length arrays in public headers. -// Keep the target-wide -Werror policy, but ignore that external-header-only -// AppleClang diagnostic after command-line warning options have been applied. -#if defined(__clang__) -#pragma clang diagnostic ignored "-Wgnu-folding-constant" -#endif - // MLX public headers. Deliberately NOT mlx/backend/metal/*: those pull in // metal-cpp and, as noted above, their entry points are not exported anyway. #include "mlx/allocator.h" diff --git a/tests/scripts/test_mlx_system_headers.py b/tests/scripts/test_mlx_system_headers.py new file mode 100644 index 00000000..1185329c --- /dev/null +++ b/tests/scripts/test_mlx_system_headers.py @@ -0,0 +1,91 @@ +#!/usr/bin/env python3 +"""Compiler-level regression tests for the MLX CMake dependency boundary.""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[2] + + +class MlxSystemHeadersTest(unittest.TestCase): + def test_dependency_module_is_shipped(self) -> None: + self.assertTrue((ROOT / "cmake" / "MLXDependency.cmake").is_file()) + + @unittest.skipUnless( + shutil.which("cmake") and shutil.which("clang++") and shutil.which("ar"), + "needs CMake, Clang, and ar", + ) + def _build(self, project_warning: bool) -> subprocess.CompletedProcess[str]: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + mlx_root = root / "mlx" + (mlx_root / "include" / "mlx").mkdir(parents=True) + (mlx_root / "lib").mkdir() + (mlx_root / "include" / "mlx" / "array.h").write_text( + "inline void mlx_warning() { const int n = 4; int values[n]; (void)values; }\n", + encoding="utf-8", + ) + library = mlx_root / "lib" / "libmlx.a" + subprocess.run(["ar", "rcs", str(library)], check=True) + warning = ( + "const int n = 4; int project_values[n]; (void)project_values;" + if project_warning + else "mlx_warning();" + ) + (root / "main.cpp").write_text( + f'#include "mlx/array.h"\nint main() {{ {warning} return 0; }}\n', + encoding="utf-8", + ) + (root / "CMakeLists.txt").write_text( + "\n".join( + [ + "cmake_minimum_required(VERSION 3.20)", + "project(mlx_system_boundary LANGUAGES CXX)", + f'list(APPEND CMAKE_MODULE_PATH "{ROOT / "cmake"}")', + "include(MLXDependency)", + f'vllm_cpp_import_mlx("{mlx_root}" "{library}")', + "add_executable(probe main.cpp)", + "target_link_libraries(probe PRIVATE vllm_cpp::mlx)", + "target_compile_options(probe PRIVATE -Wall -Wextra -Werror)", + ] + ), + encoding="utf-8", + ) + env = dict(os.environ) + env["CXX"] = shutil.which("clang++") or "clang++" + configured = subprocess.run( + ["cmake", "-S", str(root), "-B", str(root / "build")], + text=True, + capture_output=True, + env=env, + check=False, + ) + if configured.returncode: + return configured + return subprocess.run( + ["cmake", "--build", str(root / "build"), "--verbose"], + text=True, + capture_output=True, + env=env, + check=False, + ) + + def test_mlx_header_warning_is_not_promoted_by_project_werror(self) -> None: + result = self._build(project_warning=False) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_project_warning_remains_fatal(self) -> None: + result = self._build(project_warning=True) + self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("gnu-folding-constant", result.stdout + result.stderr) + + +if __name__ == "__main__": + unittest.main()