Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenElm 270M 4-bit crashes on 2018 iPad Pro (A12X Bionic) #72

Open
DePasqualeOrg opened this issue May 13, 2024 · 7 comments
Open

OpenElm 270M 4-bit crashes on 2018 iPad Pro (A12X Bionic) #72

DePasqualeOrg opened this issue May 13, 2024 · 7 comments

Comments

@DePasqualeOrg
Copy link
Contributor

When I try to generate output with OpenElm 270M on a 2018 iPad Pro, the LLMEval demo app crashes with the following error:

Compiler failed to build request
libc++abi: terminating due to uncaught exception of type std::runtime_error: [metal::Device] Unable to load kernel rmsfloat16
Encountered unlowered function call to air.simd_sum.f32

It works on my iPhone 13 and on my M3 MacBook Pro, so perhaps there's an issue with the A12X Bionic chip?

@davidkoski
Copy link
Collaborator

Possibly. This issue has some discussion on the subject:

I have used it on an iPhone 12 which has an A14 (Apple7 metal support). A12 series is Apple5 so there are certainly a bunch of features missing.

My guess is this one is "SIMD-scoped reduction operations" which is first supported in Apple7.

@awni
Copy link
Member

awni commented May 13, 2024

Yea it looks like it. Since we won't support old devices without SIMD ops, maybe the best thing to do is disable compilation for older GPU families? I think it makes sense to draw the line at Apple 7, since it's been tested (and it corresponds to M1).

@davidkoski
Copy link
Collaborator

I don't know if we can limit compilation. I think we can target different gpu families in the App Store :-)

Maybe a runtime check to at least print a warning?

@awni
Copy link
Member

awni commented May 13, 2024

We might be able to static assert on __METAL__ which is only set when using Metal 3.0:

From the metal spec:

__METAL__ // Set if compiled with the unified Metal language
// Set with -std=metal3.0 or above.

From the feature set tables:

Screenshot 2024-05-13 at 8 55 26 AM

Presumably if you build for A12 then __METAL__ will not be set by the Metal compiler so the build should break.

@davidkoski
Copy link
Collaborator

I will have to play around with that -- the same binary is used on all platforms. The only check I know of is the minimum OS version.

@awni
Copy link
Member

awni commented May 13, 2024

I was thinking this would in MLX core rather than in the Swift front-end. E.g. just a static assert in one of the metal header files (like utils.h). I think you are right that there is no environment variable for the GPU family. A runtime warning is workable, but compile is nicer if we can finagle it.

I added this to kernels/utils.h. If you have an older device, maybe you can check that it correctly breaks the build for Apple GPUs <= A13. I don't have on easily accessible to test on.

#ifndef __METAL__
static_assert(
    false,
    "Building the Metal back-end requires an Apple GPU supporting Metal >= 3.0");
#endif

@DePasqualeOrg
Copy link
Contributor Author

I can test it on my iPad, if you let me know what package/commit I need to import.

Screenshot 2024-05-13 at 18 43 29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants