Skip to content

Metal logging #2904

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.

Sign up for GitHub

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

Jump to bottom
Open
CC-Yeh wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Metal logging #2904

CC-Yeh wants to merge 5 commits into from
+88 −2

Conversation

@CC-Yeh
Copy link
Contributor

@CC-Yeh CC-Yeh commented Dec 12, 2025

Proposed changes

  • Add -fenable-metal-logging and setEnableLogging(true)(for JIT) to enable os_log for debug build.
  • Add two examples using os_log.

Checklist

Put an x in the boxes that apply.

  • I have read the CONTRIBUTING document
  • I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the necessary documentation (if needed)

@CC-Yeh CC-Yeh mentioned this pull request Dec 12, 2025
Closed
4 tasks
@CC-Yeh CC-Yeh changed the title Metal logger Metal logging Dec 12, 2025
awni
awni approved these changes Dec 17, 2025
View reviewed changes
Copy link
Member

@awni awni left a comment

Choose a reason for hiding this comment

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

Very nice, thank you!

@awni awni force-pushed the metal_os_log branch 2 times, most recently from b50c82a to 32c1a60 Compare December 17, 2025 20:55
awni
awni reviewed Dec 17, 2025
View reviewed changes
mlx/backend/metal/kernels/binary_ops.h Outdated
#include <metal_logging>
#include <metal_math>

constant os_log logger("mlx", "binary_ops");
Copy link
Member

@awni awni Dec 17, 2025

Choose a reason for hiding this comment

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

The test failed building for macOS 14 cause this API is not available. It might make sense to make a stub for lower version so we don't have to use ifdef everywhere.

Copy link
Contributor Author

@CC-Yeh CC-Yeh Dec 17, 2025

Choose a reason for hiding this comment

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

Thanks — I added a no-op os_log stub for older macOS/SDKs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@awni awni awni approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@CC-Yeh @awni