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

fix: metal on conda on a13 and higher #4483

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Leikoe
Copy link
Contributor

@Leikoe Leikoe commented May 9, 2024

metal not compiling successfully on conda is due to a wrong compile requestType. This can be fixed the following ways:

  • compile with a fairly recent libSystem.B.dylib
  • compiling a dynamic lib
  • patch Metal
  • compiling a executable library (as we do right now) + linking to any dynamic libary

This fix is using the last way. It will work on any mac which supports dynamic library and will only trigger then. For older devices it won't change anything.

@Leikoe
Copy link
Contributor Author

Leikoe commented May 9, 2024

fixes on apple silicon #2226

@geohot
Copy link
Collaborator

geohot commented May 9, 2024

Bad spacing. Don't use globals.

@@ -11,10 +11,20 @@ def wait_check(cbuf: Any):
if (error := cbuf.error()) is not None:
raise RuntimeError(error)

def get_workaround_lib(device):
options = Metal.MTLCompileOptions.new()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad indent-width

@geohot
Copy link
Collaborator

geohot commented May 9, 2024

Can we submit this request with the correct requestType directly instead of using this hack?

@Leikoe
Copy link
Contributor Author

Leikoe commented May 9, 2024

No, I looked for other ways but apple expects you not to use libraryDataContents and therefore store incomplete library data on some versions. The only way to do it properly is either to recompile and pray for system cache or use things with very limited support (BinaryArchive).

Copy link
Contributor

Changes

Name                             Lines    Diff    Tokens/Line    Diff
-----------------------------  -------  ------  -------------  ------
tinygrad/runtime/ops_metal.py      110     +10           11.8    -0.1


total lines changes: +10

@Leikoe
Copy link
Contributor Author

Leikoe commented May 10, 2024

If you were to call private C++ apis from Metal internals you could send the proper requestType but that wouldn't be supported everywhere, this hack is not as bad.

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.

None yet

3 participants