Skip to content

Fix C++ benchmarks failing to build on overloaded astype - #4025

Merged
zcbenz merged 1 commit into
ml-explore:mainfrom
ayaangazali:fix-cpp-benchmarks-build
Aug 6, 2026
Merged

zcbenz merged 1 commit into
ml-explore:mainfrom
ayaangazali:fix-cpp-benchmarks-build

Conversation

@ayaangazali

Copy link
Copy Markdown
Contributor

Proposed changes

The C++ benchmarks do not compile:

$ cmake -B build -DMLX_BUILD_BENCHMARKS=ON && cmake --build build
benchmarks/cpp/single_ops.cpp:31:3: error: no matching function for call to 'time_fn'
   31 |   TIMEM("mx::float32 to mx::int32", mx::astype, a, mx::int32, device);
benchmarks/cpp/time_utils.h:26:8: note: candidate template ignored: couldn't infer template argument 'F'
   26 | double time_fn(F fn, Args&&... args) {

mx::astype has two overloads (ops.h:51 and ops.h:52), so the bare name cannot be deduced as the template argument of time_fn. There are 11 of these across single_ops.cpp and irregular_strides.cpp, and they are the only errors in the target.

Wrapped the three argument form in a small lambda per file, which is the same style the rest of these files already use for ops that need adapting, and left the call sites otherwise unchanged.

With that, MLX_BUILD_BENCHMARKS=ON builds all four binaries and they run:

Timing (mx::float32 to mx::int32) astype ... 0.079649 msec
Timing (1D strided) astype ... 0.14269 msec

Since the benchmarks default to OFF, this looks like it went unnoticed for a while rather than being a recent regression.

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)

(benchmark sources, so no test; verified by building with benchmarks enabled and running both binaries)


honest note: freshman contributor and Claude Code helps me hunt, but i found this by turning the benchmarks on and building them, read the actual overload resolution error, checked ops.h to confirm astype really is overloaded, and then reran both binaries to make sure they produce numbers rather than just compiling. built CPU only here, so i have not exercised the Metal paths of these benchmarks.

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice catch!

@zcbenz
zcbenz merged commit 4462e87 into ml-explore:main Aug 6, 2026
28 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Aug 18, 2026
1 task done
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.

2 participants