Skip to content

Add metal precompilation #2906

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add metal precompilation #2906

wants to merge 1 commit into from

Conversation

ivarflakstad
Copy link
Member

To improve the maintainability of candle-metal-kernels I think it is about time we introduce a build.rs to the project.
This won't necessarily affect performance, but it will let us reuse components between kernels. Like get_strided_index, mlx's bfloat16 impl (when there is no native bfloat), etc.

Starting with reduce kernels and iteratively adding more to candle.metallib.

Note: When running build.rs I got an exciting new error.

xcrun: error: unable to find utility "metal", not a developer tool or in PATH

I solved it by simply running sudo xcode-select --switch /Applications/Xcode.app/Contents/Developer.
This should be documented somehow - but where? Should I add it to the error description? Or in the README.md perhaps?

@zackangelo
Copy link
Contributor

Just a heads up, I wrote a build.rs script a while back to do this:
#2335

@EricLBuehler
Copy link
Member

Note: When running build.rs I got an exciting new error.

xcrun: error: unable to find utility "metal", not a developer tool or in PATH

I solved it by simply running sudo xcode-select --switch /Applications/Xcode.app/Contents/Developer.
This should be documented somehow - but where? Should I add it to the error description? Or in the README.md perhaps?

Perhaps both. Adding it to the error would probably be the most helpful, as all downstream users will see it.

@ivarflakstad
Copy link
Member Author

ivarflakstad commented Apr 22, 2025

Just a heads up, I wrote a build.rs script a while back to do this:

#2335

Nice, thanks for sharing.
(Beat you to it ~1 year ago though 😉. Ended up not using it because it didn't improve performance)

@zackangelo
Copy link
Contributor

Just a heads up, I wrote a build.rs script a while back to do this:
#2335

Nice, thanks for sharing. (Beat you to it ~1 year ago though 😉. Ended up not using it because it didn't improve performance)

lol march to july is a whole year now? :)

@ivarflakstad
Copy link
Member Author

lol march to july is a whole year now? :)

No, I meant march to april. ~1 year ago since I did the same as I am doing now.
I'm not trying to be competitive or say "I did it first 😤 " etc hehe. I wanted to avoid a situation where you felt I had seen your impl, rewritten it in my style, and made a PR without acknowledging you at all. That would be shitty, so wanted to explain that it is not the case 🙇

@zackangelo
Copy link
Contributor

lol march to july is a whole year now? :)

No, I meant march to april. ~1 year ago since I did the same as I am doing now. I'm not trying to be competitive or say "I did it first 😤 " etc hehe. I wanted to avoid a situation where you felt I had seen your impl, rewritten it in my style, and made a PR without acknowledging you at all. That would be shitty, so wanted to explain that it is not the case 🙇

Oh sorry, was just joking a bit, didn't mean to intend any offense! 😅 No worries at all, and if there's anything I can help with let me know. And of course feel free to borrow any bits in my PR you might find useful.

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

Successfully merging this pull request may close these issues.

3 participants