Skip to content

Use common dispatch for compiled constants - #4129

Merged
zcbenz merged 2 commits into
ml-explore:mainfrom
PhysicistJohn:dispatch-compiled-constant
Aug 11, 2026
Merged

zcbenz merged 2 commits into
ml-explore:mainfrom
PhysicistJohn:dispatch-compiled-constant

Conversation

@PhysicistJohn

@PhysicistJohn PhysicistJohn commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Replace print_constant's exhaustive dtype switch with dispatch_all_types.
Compile-time capability branches retain the existing bool, byte-integer,
integer, floating, and complex formatting paths for every current dtype.

This removes sixteen net lines from one function.

Testing

  • Release CPU-only build with C++20 and warnings as errors
  • Exact compiled.cpp Release translation-unit replay with warnings as errors;
    the replay matched the stored 41,120-byte object with a 14,836-byte text
    section
  • Baseline/candidate generated-text parity for all 14 current dtypes, including
    the int8 and uint8 numeric formatting paths
  • Native compile/JIT suite (24/24 cases, 117/117 assertions)
  • Full native C++ suite (247/247 cases, 3,326/3,326 assertions)
  • Release object and static library each shrink by 512 bytes; defined global
    object and library symbol-name lists are unchanged, while the now-unused
    std::runtime_error(char const*) import is removed
  • Same-file overlap check against experimental [Experiment] ROCm backend #2300: it changes
    compiled_check_contiguity and its call in
    compiled_collapse_contiguous_dims, not print_constant; this change adds
    no new conflict to [Experiment] ROCm backend #2300's existing current-main conflict
  • clang-format and git diff --check

Comment thread mlx/backend/common/compiled.cpp Outdated
throw std::runtime_error("Unsupported constant type");
}
});
if (!printed) {

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.

I think this check is redundant?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks — yes, good catch. Given the array dtype invariant, dispatch_all_types covers every current array::dtype() value, so the outer sentinel was redundant. I removed it and reran the 14-dtype text parity, compile/JIT tests (24 cases, 117 assertions), and full C++ suite (247 cases, 3,326 assertions); all pass.

@zcbenz
zcbenz merged commit 63e98d8 into ml-explore:main Aug 11, 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