Skip to content

Vectorize nested multiply-accumulate loops into @fmac* DSD operations - #72

Open
ycmath wants to merge 1 commit into
spcl:mainfrom
ycmath:mac-loop-vectorization
Open

ycmath wants to merge 1 commit into
spcl:mainfrom
ycmath:mac-loop-vectorization

Conversation

@ycmath

@ycmath ycmath commented Aug 26, 2026

Copy link
Copy Markdown

Implements the targeted MAC-loop vectorization from #69, incorporating the staging agreed there: this PR delivers the base vectorization together with asks (2), (3), and (5); asks (1) and (4) are deferred to a follow-up PR pending the increment_dsd semantics question raised in the issue thread.

What this does

emit_for now recognizes the two-variable nested multiply-accumulate pattern

for (k, l) in [0:K, 0:L]:
    Z[k] = Z[k] + A[k*Ck + l*Cl + C0] * X[f(l)]

and lowers it to a strided base DSD over A plus a per-l @increment_dsd_offset and one @fmac* per column, instead of scalar loops — the same idiom as the handwritten CSL GEMV. On samples/spatial/blas/gemv.sptl this measured 4.5–7.7× (details in #69).

  • Loop order (ask 2): operand roles are derived from the accumulator index — the loop variable indexing Z with coefficient 1 takes the k role, the other is the reduction variable — so for (l, k) vectorizes identically to for (k, l). Per-element accumulation order over l is preserved bitwise in both orders.
  • Type generality (ask 3): dispatch mirrors FMADSDOp._as_csl: all-f16 → @fmach, f32 accumulate with f16 scalar → @fmachs, all-f32 → @fmacs; the @increment_dsd_offset element type follows A. Unsupported combinations fall back to scalar loops.
  • No separate flag (ask 5): the vectorizer sits behind --disable-dsd like all other DSD detection.

Conservative by construction — falls back to scalar code on: aliasing of Z with A or X (DSD ops reorder reads relative to the sequential loop), non-affine indices, non-unit steps, nonzero k start, or any dtype combination outside the table above.

Tests

tests/spatial_ir/test_mac_vectorization.py: 11 tests covering the positive matrix (both loop orders × f32/f16/mixed dtypes) and negative cases (aliasing in both orders, non-affine index, accumulator mismatch, unsupported dtypes, --disable-dsd). Full suite: 386 passed, 12 skipped on this branch.

@increment_dsd_offset is available from SDK 1.x (Cerebras' own csl-examples v1.4.0 cholesky uses it), so generated code stays compatible with the SDK 1.4 pinned in CI; the f16 form of the type parameter was additionally verified to compile with cslc.

🤖 Generated with Claude Code

Detect the two-variable nested MAC pattern
  Z[k] = Z[k] + A[affine(k, l)] * X[f(l)]
in emit_for and lower it to a strided base DSD over A plus a per-l
@increment_dsd_offset and the dtype-matched @FMac* builtin, instead of
scalar loops. On samples/spatial/blas/gemv.sptl this gives 4.5-7.7x
(issue spcl#69).

Per the discussion in spcl#69:
- Loop-header order is irrelevant: the variable indexing the accumulator
  takes the k role, the other is the reduction variable (ask 2).
- Dtypes dispatch like FMADSDOp._as_csl: @fmach (f16), @fmachs
  (f16 multiply / f32 accumulate), @fmacs (f32) (ask 3).
- No separate flag: the vectorizer is gated behind --disable-dsd like
  all other DSD detection (ask 5).

Conservative by construction: falls back to scalar loops on aliasing of
Z with A or X, non-affine indices, unsupported dtype combinations,
non-unit steps, or a nonzero k start. Arbitrary nesting depth and
generalization to other DSD ops (asks 1 and 4) are deferred to a
follow-up PR pending the increment_dsd semantics question.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ycmath

ycmath commented Sep 13, 2026

Copy link
Copy Markdown
Author

Note for review: #75 was an accidental duplicate of this PR from a parallel workflow on my side and is now closed — this PR is the canonical one.

It is based on 7be46fb but merges cleanly onto current main (post-#70); on the merged tree the full suite is 386 passed / 12 skipped. On that merged tree the generated CSL also compiles with cslc (SDK 2.10) — both the samples/spatial/blas/gemv.sptl output and a minimal mixed-dtype kernel whose @fmachs form (f32 accumulator and DSD operand, f16 scalar) is the one this PR emits. Note that the branch needs the merge to compile under SDK 2.10 at all: its pre-#70 base still emits param memcpy_params: comptime_struct;, which 2.10 rejects independently of this change.

One @fmachs form is still left on the table: the SDK's own csl-libs/dsd_ops.csl also permits an f16 DSD operand with an f32 accumulator, which this pass declines and lowers to a scalar loop. Happy to add that (plus integer-dtype fallback tests) here or in the follow-up, whichever you prefer.

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.

Nested multiply-accumulate loops lower to scalar code; targeted @fmacs vectorization gives 4.5–7.7× on samples/spatial/blas/gemv.sptl

1 participant