Skip to content

refactor(ops): migrate core/elementwise primitives into kernels/ops/ (completes wave 1) - #18

Merged
TheTom merged 3 commits into
devfrom
ek/kernel-reorg-ops
Jun 22, 2026
Merged

refactor(ops): migrate core/elementwise primitives into kernels/ops/ (completes wave 1)#18
TheTom merged 3 commits into
devfrom
ek/kernel-reorg-ops

Conversation

@ekryski

@ekryski ekryski commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

What

Final wave-1 kernel-family port: moves the core / elementwise primitive
kernels out of the legacy mlx/ + ffai/ split into kernels/ops/ per
docs/specs/KERNEL_CONSOLIDATION_PLAN.md. With this, wave 1 is complete
(rope · norm · sampling · ops).

Changes

  • mlx/{arange,arg_reduce,binary,binary_two,copy,gather_axis,hadamard,hadamard_m, indexing,logsumexp,random,reduce,scan,scatter_axis,strided,ternary,unary}
    kernels/ops/ (already mt_*; these are inventory-registered, so there were
    no path-import sites to fix beyond one snapshot test).
  • ffai/{axpy_scalar_inplace,clamp_scalar,gather} + mt_vector_add.rs (→
    vector_add.rs) → kernels/ops/ with ffai_mt_ prefixes.
  • Dedup: deleted ffai/arg_reduce.rsffai_argmax was a byte-for-byte
    duplicate of mt_argmax (verified line-by-line), so it's dropped, not moved.
  • fence.rs moves but stays intentionally undeclared — device/system memory
    fences and atomics have no #[kernel] DSL representation (the "1 out of scope"
    op in the audit); the file is kept for its implementation notes.
  • Updated the steel_msl_snapshots import, the op_group / name-filter unit-test
    fixtures, the plan (ops done, wave 1 complete), and the KERNEL_AUDIT rows.

Generated MSL per kernel is unchanged — only module paths and inventory names.

Verification

  • cargo build -p metaltile-std + cargo clippy --workspace --all-targets clean.
  • All ops kernel tests pass via tile test (binary, unary, ternary/select, copy,
    strided, arange, random, reduce, argmax/argmin, scan, indexing, gather(_axis),
    scatter_axis, hadamard(_m), logsumexp, clamp, axpy, vector_add).
  • steel_msl_snapshots integration test passes.

Note on stacking

Stacked on sampling (#17) → norm (#16) → conv+rope (#15) → docs (#14); the
diff includes those until they merge. Each family lands as its own PR.

@github-actions github-actions Bot added the ignore-for-release Hidden from release notes label Jun 12, 2026
@ekryski
ekryski force-pushed the ek/kernel-reorg-ops branch 2 times, most recently from d078f11 to 1fa8029 Compare June 12, 2026 23:21
@TheTom

TheTom commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Wave 1 complete and the increment is the cleanest of the stack: unary.rs is a 100% similarity rename, and the steel_msl_snapshots.rs change is import-path only, which is exactly the evidence the consolidation plan's gate asks for (generated MSL unchanged, only module paths moved).

One stack-management note for all four PRs: they all base on dev, so the GitHub diffs for #16/#17/#18 include their predecessors and the +/- counts overstate each PR by ~23k lines. Two options: retarget each PR's base to its predecessor branch for true incremental diffs, or just merge strictly in order 15 -> 16 -> 17 -> 18 (GitHub auto-retargets the survivors after each merge). Either works, merging in order is less clicking. After #18 lands, mlx/ is down to the wave-2/3 families (steel, block-scaled, quantized), which sets up the next stack nicely.

@ekryski
ekryski force-pushed the ek/kernel-reorg-ops branch from 1fa8029 to 78fc51f Compare June 12, 2026 23:34
@ekryski

ekryski commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Wave 1 complete and the increment is the cleanest of the stack: unary.rs is a 100% similarity rename, and the steel_msl_snapshots.rs change is import-path only, which is exactly the evidence the consolidation plan's gate asks for (generated MSL unchanged, only module paths moved).

One stack-management note for all four PRs: they all base on dev, so the GitHub diffs for #16/#17/#18 include their predecessors and the +/- counts overstate each PR by ~23k lines. Two options: retarget each PR's base to its predecessor branch for true incremental diffs, or just merge strictly in order 15 -> 16 -> 17 -> 18 (GitHub auto-retargets the survivors after each merge). Either works, merging in order is less clicking. After #18 lands, mlx/ is down to the wave-2/3 families (steel, block-scaled, quantized), which sets up the next stack nicely.

Agreed. We'll just merge in order. Was just rebasing and ensuring builds are all green.

Working on wave 2 and 3 families. Will follow similar PR structure.

@TheTom

TheTom commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

@ekryski clean; ffai_argmax dedup is legit (== mt_argmax). Body omits two real items in the diff: the mlx/{swiglu,clamped_swiglu,fused_gate_activation}kernels/ops/gated_activation.rs merge (names preserved), and the protocol.rs/harness.rs test-fixture edits for ffai_vector_addmt_vector_add. Worth a body line so the cross-crate hunks aren't a surprise. Approve.

ekryski added 3 commits June 22, 2026 16:24
Completes wave 1: moves the core/elementwise primitive kernels out of the
legacy mlx/ + ffai/ split into kernels/ops/ per the consolidation plan.

- mlx/{arange,arg_reduce,binary,binary_two,copy,gather_axis,hadamard,
  hadamard_m,indexing,logsumexp,random,reduce,scan,scatter_axis,strided,
  ternary,unary} -> kernels/ops/ (already mt_*; inventory-registered, so no
  import sites to update beyond one snapshot test)
- ffai/{axpy_scalar_inplace,clamp_scalar,gather} + mt_vector_add.rs (renamed
  vector_add.rs) -> kernels/ops/ with ffai_ -> mt_ prefixes
- delete ffai/arg_reduce.rs: ffai_argmax was a byte-for-byte duplicate of
  mt_argmax (verified) -> real dedup
- fence.rs moves but stays intentionally undeclared (device/system fences have
  no DSL representation — the '1 out of scope' op)

Updated the steel_msl_snapshots import, the op_group/name-filter test fixtures,
the plan (ops done, wave 1 complete) and KERNEL_AUDIT rows.

cargo build + workspace clippy clean; all ops kernel tests + steel_msl_snapshots
pass.
…probe

Finishes rehoming the last mlx/ stragglers:

- gated activations: fold mlx/{swiglu,clamped_swiglu,fused_gate_activation}.rs
  (4 kernels) into one kernels/ops/gated_activation.rs. Names unchanged
  (mt_swiglu, mt_clamped_swiglu, mt_fused_gate_gelu,
  mt_fused_gate_clipped_swiglu); ops/ is the activation home (mt_silu, which
  mt_swiglu cross-kernel-calls, lives in ops/unary.rs). 3 files -> 1.
- mlx/sgload_smoke.rs -> probe/simdgroup_load_probe.rs; mt_sgload_smoke ->
  mt_simdgroup_load_probe. It's a HW-intrinsic validation kernel (sibling of
  probe/mma_layout_probe + mpp_matmul_smoke), so probe/ is its real home, not
  ops/. Renamed for what it does (validates the Op::SimdgroupLoad codegen path
  via a byte-exact round-trip). Test file renamed to match.

Updated KERNEL_AUDIT (swiglu/fused rows) + the plan's ops listing. cargo build
+ clippy clean; gated-activation kernel tests + the probe integration test pass.
…t the probe convention

Makes the probe/ module consistent: all three validation kernels now use the
_probe suffix (mma_layout_probe, simdgroup_load_probe, mpp_matmul_probe).

- probe/mpp_matmul_smoke.rs -> mpp_matmul_probe.rs; mt_mpp_matmul_smoke ->
  mt_mpp_matmul_probe; integration test tests/mpp_matmul_smoke.rs renamed to
  match.
- STYLE_GUIDE §1: document the convention — a kernel that validates a codegen
  path / HW intrinsic is a 'probe' (mt_<thing>_probe, in probe/, outside
  kernels/<family>/). '_smoke' is reserved for *test* code (backend bring-up
  tests, codegen-sanity #[test]s), which stays test-side.
- probe/mod.rs: add a module doc stating the same.

No other kernels use the _smoke naming (the remaining *_smoke are legit tests:
cuda/hip/vulkan backend bring-up + codegen-sanity test fns — left as-is).

cargo build + clippy clean; probe integration + codegen unit tests pass.
@TheTom
TheTom force-pushed the ek/kernel-reorg-ops branch from fa62466 to 0c1a070 Compare June 22, 2026 21:24
@TheTom
TheTom merged commit 22e756e into dev Jun 22, 2026
8 checks passed
@ekryski
ekryski deleted the ek/kernel-reorg-ops branch July 13, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ignore-for-release Hidden from release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants