Skip to content

Bridge generic complex storage into Metal FFT - #6

Closed
PhysicistJohn wants to merge 1 commit into
integration/reduced-complex-foundations-v2from
draft/reduced-complex-storage-bridge
Closed

Bridge generic complex storage into Metal FFT#6
PhysicistJohn wants to merge 1 commit into
integration/reduced-complex-foundations-v2from
draft/reduced-complex-storage-bridge

Conversation

@PhysicistJohn

@PhysicistJohn PhysicistJohn commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Note

Fork-only review PR. This is not an upstream submission.

Context

This is the narrow integration point for two foundations under upstream review:

  • ml-explore/mlx#3969 makes Metal C2C FFT arithmetic and I/O generic over the scalar lane (611e080).
  • ml-explore/mlx#3970 makes complex_t<T> lane-generic while preserving complex64_t and existing entry points (7ea2b9a).

The synthetic base, integration/reduced-complex-foundations-v2, merges the exact current heads of those two PRs without other source changes. This PR adds one commit that connects them: FFTValueTraits can load and store complex_t<T> while the FFT retains its lane-typed vector representation internally.

Change

  • Add FFTValueTraits<complex_t<T>> with a floating-lane constraint, an exact two-lane layout check, and explicit load/store conversion.
  • Add a build-time Metal regression that instantiates the adapter for half and bfloat16_t.
  • Keep the regression AIR test-only; it is not linked into or installed with mlx.metallib.

Precedent

Reduced-precision complex FFT storage is established elsewhere:

Those implementations have backend-specific constraints. This patch adds only MLX's internal storage bridge; it does not claim public API parity.

Scope

  • Three files, 52 added lines, one commit.
  • No public dtype, API, promotion, or host-dispatch changes.
  • No production half/BF16 kernel instantiations.
  • No changes to existing float C2C or packed RFFT/IRFFT entry points.
  • No FP8 work.

Validation

  • The probe fails to compile on the dependency base and passes with this adapter.
  • Native Metal 320 compilation with warnings as errors emits both fft_complex_storage_half and fft_complex_storage_bfloat16.
  • LLVM retains { half, half } and { bfloat, bfloat } storage with zero fpext and zero fptrunc instructions.
  • Static and JIT CMake configurations produce byte-identical probe AIR.
  • Base and patched production FFT AIR and LLVM are byte-identical when compiled from the same path.
  • Full native build and serial CTest: 261/261 passed.
  • Repository hooks and git diff --check pass.

The shipped mlx.metallib is unchanged at 131,007,704 bytes, SHA-256 8b7a8614751b1d41cb09e87cb6cf2ff7d35d2bf7b1c5881ecf842d253412dd19; neither test symbol is exported. A measured future static C2C matrix would add about 0.758% for half, 0.758% for BF16, or 1.516% for both, but this PR pays none of that cost.

Runtime proof outside this diff

The preserved fork-only proof commit exercises Stockham, Rader, Bluestein, and four-step with half and BF16 storage. Packed storage is 4 bytes per complex value instead of 8, and the audited FFT lanes remain native reduced precision. Half measured 1.13-2.30x float throughput across the four representative planners; BF16 measured 0.86-2.12x. The BF16 Bluestein regression is deliberately not hidden and remains a separate planner-constant optimization rather than part of this storage adapter.

Public dtype semantics, CPU support, accuracy policy, production dispatch coverage, and planner-owned reduced-precision constants remain separate reviewable decisions after the foundations land.

Teach FFTValueTraits to load and store complex_t<T> while preserving lane-typed vector arithmetic.

Add a compile-only Metal regression for half and bfloat16 adapters without adding production kernel instantiations or changing the shipped metallib.
@PhysicistJohn

Copy link
Copy Markdown
Owner Author

Superseded: the storage bridge is now folded into ml-explore#3981.

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.

1 participant