BUG: Fix missing bf16 Metal kernels on Apple Silicon - #174
Conversation
Add patch management for llama.cpp during build process.
This patch pins the Metal Shading Language (MSL) version for shader compilation to avoid runtime failures related to bfloat and tensor API support.
Update revert_llamacpp_patches function to modify mtime of touched files after reverting patches.
This file contains unit tests for the build-time patching mechanism of llama.cpp. It tests the application and reversion of patches in a temporary git repository.
This file contains regression tests for Metal bf16 kernel support on Apple Silicon, ensuring that models with BF16 tensors load correctly in both standard and old SDK environments.
There was a problem hiding this comment.
Code Review
This pull request introduces a build-time patching mechanism for the vendored llama.cpp submodule, applying local hotfixes (such as pinning the Metal Shading Language version to ensure bf16 kernel support on Apple Silicon) and reverting them afterward to keep the working tree clean. It also adds comprehensive unit and regression tests. The review feedback suggests several robustness improvements: using raw MSL version values to avoid compilation failures on older macOS SDKs, wrapping the patch application in a try...except block to revert applied patches if one fails, guarding against None values for LIBDIR in tests, and asserting on subprocess return codes to improve test diagnostics.
| include = sysconfig.get_paths()["include"] | ||
| libdir = sysconfig.get_config_var("LIBDIR") |
There was a problem hiding this comment.
In some Python environments or virtual environments, sysconfig.get_config_var("LIBDIR") can return None. If libdir is None, compiling the host executable with clang will fail. Adding a guard to skip the test if LIBDIR is not available makes the test suite more robust.
include = sysconfig.get_paths()["include"]
libdir = sysconfig.get_config_var("LIBDIR")
if not libdir:
pytest.skip("LIBDIR is not available in sysconfig")Pin the Metal Shading Language (MSL) version for shader compilation to ensure compatibility with hardware requirements.
Add checks for Metal bf16 kernel support on Apple Silicon and ensure proper error handling for bfloat fallback scenarios.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Refactor patch application logic to handle exceptions and revert applied patches if an error occurs.
Pin the Metal Shading Language (MSL) version for shader compilation to ensure compatibility with macOS GPU requirements.
Pin the Metal Shading Language (MSL) version for shader compilation to ensure compatibility with devices lacking tensor API support.
Updated pytest command to include the '-s' option for output and added a step to dump macOS crash reports if pytest crashes.
Updated regression tests for Metal bf16 kernel support to reflect correct Apple Silicon models and ensure proper handling of BF16 operations across different SoC generations.
|
This PR is very similar to the fix used upstream. You may wish to compare the two patches for validity. |
Thanks, I’ll compare the two patches. |
This patch updates the Metal Shading Language (MSL) version handling in the ggml-metal-device.m file to ensure compatibility with the required features. It modifies the way MSL version is set based on device capabilities and refactors the library compilation process.
Problem
Loading a model with BF16 tensors (e.g. gemma-4) on a bfloat-capable Apple GPU fails during warmup with:
Root cause
The wheel builds with
GGML_METAL_EMBED_LIBRARY=ON, so the Metal shader source is embedded and JIT-compiled at runtime. llama.cpp injectsGGML_METAL_HAS_BF16=1whenever the GPU reports bfloat support, butggml-metal.metalre-checks that decision itself:llama.cpp never sets
MTLCompileOptions.languageVersion, so Metal derives the default shading language version from theLC_BUILD_VERSIONof the host executable — the user's python interpreter, not our extension. Interpreters linked against old SDKs (conda, python.org ≤ 3.12) yield MSL 2.x, where the guard above silently strips all 55 bf16 kernels from a library that still compiles successfully while the C++ side keepshas_bfloat = true. The model loads; the first BF16 matmul dies with "was not found in the library".The same mechanism disables the M5 tensor-API path under old-SDK hosts:
<metal_tensor>/ MetalPerformancePrimitives require MSL 4.0, so the capability probes fail even on hardware that supports them.Fix
patches/llama.cpp/0002-metal-pin-msl-language-version.patch— one file (ggml-metal-device.m), applied at build time byscripts/build.py:ggml_metal_device_msl_version_min()maps device props to the MSL version its enabled features need:has_tensor→ 4.0,has_bfloat→ 3.1, else 0 (host default untouched). Rawmajor << 16 | minorvalues keep the patch buildable against older Xcode SDKs.ggml_metal_compile_source()— a single shared helper used by both JIT call sites (ggml_metal_library_init,ggml_metal_library_init_from_source). It applies the pin and wraps the compile in@try/@catch: an MSL version the linked framework rejects can raise an uncaughtNSExceptioninstead of populatingNSError*, which wouldabort()the whole python process — now it is logged and treated as an ordinary compile failure.bfloatkernel through the same helper. If the environment genuinely cannot compile what the device claims to support,has_bfloatis disabled with a warning and BF16 ops fall back to the CPU backend — instead of crashing at the first BF16 op.Per-chip effect
has_tensor/has_bfloatGGML_METAL=OFFor no MetalNotes:
GGML_METAL_BF16_DISABLE=1remains as a manual escape hatch.ver_min == 0→ host default preserved).Testing
tests/test_metal_bf16.py(runs on Apple Silicon CI):LC_BUILD_VERSIONis rewritten to SDK 11.0 (vtool), reproducing the conda/python.org failing scenario.Notes for reviewers
0001-metal-pin-msl-language-version.patch, which pinned 3.1 only forhas_bfloat && !has_tensor(leaving the M5 path unprotected) and lacked the exception guard.