Report the MLX backend as unavailable on the iOS simulator - #22336
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22336
Note: Links to docs will display an error until the docs builds have been completed. ❌ 3 New Failures, 2 Unrelated FailuresAs of commit 9ce78e1 with merge base 2b3a32d ( NEW FAILURES - The following jobs have failed:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following job failed but was present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
|
Thanks, the comment finding was a real error on my part. The comment was wrong about MLX and is fixed. You are right that MLX does not require the shared Both documentation pages are fixed. The iOS page listed the MLX framework with no exception while Two I looked at and am leaving: The include. I checked whether dropping it silently disables the guard, since that would be the The simulator slice of the Metal kernel library. Fair point that the build still produces it, the |
500efa7 to
11f9377
Compare
|
Third pass. All the documentation findings are fixed, and the test gap is closed. A correction to something I said last round. An earlier review told me MLX null-checks the heap and The test gap is real and now closed. You are right that nothing would notice if the guard were The iOS page was contradicting itself and that was my fault. It now keeps the plain framework entry, The description now covers the documentation changes, says the iOS device rows reflect what the build Two follow-ups I would rather not fold in: adding |
d893b71 to
d7eb824
Compare
| Result<FileDataLoader> loader = FileDataLoader::from(program_path()); | ||
| ASSERT_EQ(loader.error(), Error::Ok); | ||
| Result<Program> program = Program::load(&loader.get()); | ||
| ASSERT_EQ(program.error(), Error::Ok); | ||
| ManagedMemoryManager mmm(kDefaultNonConstMemBytes, kDefaultRuntimeMemBytes); | ||
|
|
||
| Result<Method> method_res = program->load_method("forward", &mmm.get()); | ||
| EXPECT_EQ(method_res.error(), Error::NotFound); |
There was a problem hiding this comment.
Done. Added EXPECT_TRUE(program->method_meta("forward")->uses_backend(StubBackend::kName)) before the load, so the test fails loudly if the fixture ever stops delegating to the stub, rather than passing NotFound for the wrong reason. This matches the sibling tests in this file (e.g. GetBackendNamesSuccess) which assert uses_backend the same way.
| | [CUDA](backends/cuda/cuda-overview.md) | Linux/Windows | GPU | NVIDIA GPU acceleration | | ||
| | [Core ML](backends/coreml/coreml-overview.md) | iOS, macOS | NPU/GPU/CPU | Apple devices, high performance | | ||
| | [MLX](/backends/mlx/mlx-overview.md) | macOS | GPU | Apple Silicon GPU (MLX) | | ||
| | [MLX](/backends/mlx/mlx-overview.md) | iOS, macOS | GPU | Apple Silicon GPU (MLX) | |
There was a problem hiding this comment.
Not new support added by this PR, but it is accurate: MLX is already built, packaged, and published for the iOS device slice. The Apple framework build runs the ios preset (PLATFORM=OS64, iOS 17), a per-platform Metal patch produces the iphoneos metallib, Package.swift ships mlx-ios.metallib, and apple.yml fails publication if that device slice is missing. The runtime has no iOS-device gate: is_available() just defers to mlx::core::metal::is_available(). The one place it genuinely broke was the simulator (no usable Metal device), which is what this PR fixes. The honest caveat is that on-device execution isn't covered by physical-device CI yet; happy to word it as experimental if you'd prefer. I've also removed iOS from the pip CMake-components table in using-executorch-cpp.md, since that table is host/wheel-scoped (Linux/macOS) like the Core ML row.
There was a problem hiding this comment.
Let's word as experimental. I've never run a model with it on iOS
There was a problem hiding this comment.
Done. Marked iOS as experimental: the backends-overview table now reads "iOS (experimental), macOS", and the MLX overview page says the iOS device path is experimental and notes that on-device execution isn't covered by CI yet. (The page already flagged the whole MLX delegate as experimental at the top.) So the docs now claim only what's actually been exercised.
Loading a model that delegates to MLX crashes the process on an iOS simulator instead of failing
cleanly. The demo app in the examples repository crashes on the MLX menu entry, and the llama and
image classifier tests crash with it.
The simulator's Metal device is real enough to build and dispatch a compute pipeline, but two of
the things MLX needs unconditionally are missing from it: `architecture` reports nothing, and a
shared storage heap is refused by an assertion inside Metal. MLX reads the first without a null
check, so it faults during device construction, and if that is bypassed it aborts on the second.
Neither is reachable as an error, so nothing downstream can report it.
`is_available()` is the interface's own answer to whether a backend can run here, and the runtime
already asks before loading a delegate, so returning false is enough to turn the crash into
`Error::NotFound`. MLX's own `is_available()` returns a constant and checks nothing.
Test plan:
The macro decides everything here, so it is checked on all three targets it affects:
simulator TARGET_OS_SIMULATOR 1, is_available() false
device TARGET_OS_SIMULATOR 0, is_available() defers to MLX
macOS TARGET_OS_SIMULATOR 0, is_available() defers to MLX
so a real GPU is unaffected. Compiling the same function with the guard disabled returns true on
the simulator again, which is the crashing path, so the guard is doing the work.
Confirmed separately that a delegate whose backend reports unavailable makes `load_method` return
`Error::NotFound` rather than crash. The include is explicit because an undefined macro is zero in
`#if`, which would make the guard silently do nothing.
This does not make MLX work on the simulator. It makes the answer honest, so a caller gets an
error it can handle.
Review found one clause of the new comment is wrong about MLX. MLX does not require the shared storage heap: its allocator skips it on a paravirtual device and every use tests for it first, so a heap it never got would not sink it. The real sequence is that Metal traps inside the heap request, so MLX never receives a value it could fall back from. Only the architecture read genuinely has no null check. Left as it was, the next reader goes looking for a missing check in MLX and finds one that is already there. The comment also now says this is a build switch rather than a probe, and what would let it go. Two documentation pages said the opposite of the code. The iOS page lists the prebuilt frameworks as working on devices and simulators and listed the MLX backend with no exception, and the MLX overview gave only Mac under target requirements without mentioning iOS at all. Both now say MLX runs on real devices and Mac but not the simulator, so the first sign of this is not a load error. Test plan: Unchanged from the previous revision, since no behaviour changed here. The guard still compiles clean with -Wall -Wextra for macOS, the iOS simulator and iOS device, and the macro still resolves to 1 only for the simulator.
d7eb824 to
db3986e
Compare
Nothing would have noticed if this guard were deleted. The only job that compiles this file for a
simulator compiles it with tests off, and it passes on the base commit too, so the guard could go and
every check would stay green.
The executor's stub backend already has a hook to install a custom availability answer, unused until
now. Installing false and loading a delegated method asserts `Error::NotFound` rather than the backend
being initialized anyway. That runs on every pull request, on Linux, with no Apple hardware, and it
fails if the check in `Method::load` goes.
On the documentation, my last change left the iOS page contradicting itself. It said the MLX framework
is for real devices and Mac only, while the same page maps a simulator platform name, force loads the
MLX library for that slice, and asks the reader to ship the simulator kernel file. The framework does
still build, ship and link there; only the runtime answer changed. So the list entry goes back to
plain, and the behaviour is a note beside the existing ones, or a reader starts pulling the simulator
slice out of their build.
Two more pages said macOS only, the backend table and the C++ component table, which are where most
people look first. Both now read iOS and macOS, matching the Core ML row above.
The iOS requirement also carries its version now. The build stops with a fatal error below iOS 17.0
and the Swift package declares iOS 17, so the number belongs on the line. The requirements list reads
as alternatives rather than a set that must all hold.
Test plan:
unavailable backend load_method returns Error::NotFound
check in Method::load removed the test fails
Guard behaviour unchanged from the previous revision: the macro is 1 only for the simulator, and the
same function with the guard disabled returns true there, which is the crashing path.
db3986e to
9ce78e1
Compare
Summary
Loading a model that delegates to MLX crashes the process on an iOS simulator instead of failing
cleanly. In the examples repository the demo app crashes on its MLX menu entry, and the llama and
image classifier tests crash with it.
The simulator's Metal device is real enough to build and dispatch a compute pipeline, but two things
MLX does during startup do not survive it:
device.architecturewhile constructing its deviceMLX has an opt-out for the heap, but it only fires when the device names itself
Apple Paravirtual device. A simulator reportsApple iOS simulator GPU, so the request goesthrough. Neither failure is reachable as an error, so nothing downstream can report it.
is_available()is the interface's own answer to whether a backend can run here, and the runtimealready asks before loading a delegate, so returning false is enough to turn the crash into
Error::NotFound. MLX's ownis_available()returns a constant and checks nothing, which is why theanswer has to come from here.
This does not make MLX work on the simulator. It makes the answer honest, so a caller gets an error it
can handle.
Documentation
Four pages disagreed with each other about where MLX runs. The MLX overview listed Mac only, while the
build has shipped an iOS slice and declared iOS 17 for some time, so the page was behind the build.
The backend table and the C++ component table also said macOS only. All three now say iOS and macOS,
with the iOS minimum version the build actually enforces.
The iOS page keeps the MLX framework in its list without an exception, because the framework still
builds, ships and links for the simulator: only the runtime answer changed. The simulator behaviour is
called out in a note beside the existing ones instead, so nobody reads the list and starts removing
the simulator slice from their build.
Test plan
A runtime test that needs no Apple hardware: the executor's stub backend already has a hook to install
a custom availability answer, unused until now. Installing
falseand loading a delegated methodreturns
Error::NotFoundrather than initializing the backend. That runs on every pull request, onLinux, and it fails if the check in
Method::loadis removed.For the guard itself, the macro decides everything, so it is checked on all three targets it affects:
A real GPU is unaffected. Compiling the same function with the guard disabled returns true on the
simulator again, which is the crashing path.
The two Metal facts above were measured directly, with a standalone Metal probe run in a booted
simulator: the device reports
Apple iOS simulator GPUwith a null architecture, and the heap requestaborts with
MTLStorageModePrivate is required for heaps, signal 6.Not covered: no model was run through MLX on a physical iPhone or iPad as part of this change. The iOS
device rows in the tables reflect what the build already ships, not a new claim measured here.
Follow-ups, not in this change
apple.ymlhas a path filter that does not listbackends/mlx, so the iOS framework job does notrun for MLX runtime changes. Adding it is one line and belongs in its own change.
deliberate for now: if the simulator gains a usable Metal device the guard goes and the file is
needed again.