Conversation
added 6 commits
September 2, 2026 01:43
Ports p4_mac_vectorization.patch (issue spcl#69) onto eaf8d1c, which added the Appliance Mode compiler (spcl#70) and refactored CLI options into a shared `codegen_options` decorator used by both `compile_spatial_ir` and the new `compile_spatial_ir_appliance`. The patch's CLI hunk no longer applied cleanly (2/3 hunks rejected) because compiler.py's option list and function signature moved; the lowering/statements.py hunks applied without conflict. Resolved by hand-adding --disable-mac-vectorization to the shared codegen_options decorator and threading disable_mac_vectorization through generate_program() and both CLI entry points (compile_spatial_ir, compile_spatial_ir_appliance), matching how --disable-dsd is already threaded through both. Baseline (upstream HEAD, no P4): 375 passed, 12 skipped. This commit: 381 passed, 12 skipped (0 regressions, +6 P4 tests).
Detect the row variable (k, appears alone with coefficient 1 in the destination index) and the reduction variable (l, everything else) from the destination index's affine decomposition instead of assuming statement.variables[0]/[1] position. Accepts both `for (k, l) in [...]` and `for (l, k) in [...]` declarations of the same MAC loop; behavior for the existing (k, l) shape is unchanged (same equality check on the affine decomposition, now checked against either candidate name instead of a hardcoded position). Verified manually with an (l, k)-order kernel: emits identical @fmacs/@increment_dsd_offset output to the (k, l) case (ad hoc script, not committed). Full suite: 381 passed, 12 skipped (unchanged from previous commit).
Replace the ad hoc base_f32() gate with a dispatch mirroring FMADSDOp._as_csl (dsd_ops.py) exactly, reusing dsd_ops._get_base_dtype for dtype resolution instead of a hand-rolled base_type/element_type lookup: - f16 matrix/vector operands, f16 accumulator -> @fmach - f32 matrix/vector operands, f32 accumulator -> @fmacs (unchanged) - f32 matrix/vector operands, f16 accumulator -> @fmachs (mixed) The @increment_dsd_offset elem_type argument (previously hardcoded `f32`) and the emitted op name are now both derived from the resolved matrix/vector dtype and dst dtype respectively, via dtype_as_csl(). f16 multiplicands with an f32 accumulator, and all integer dtypes, are intentionally NOT vectorized: FMADSDOp itself has no dispatch branch for f16-mul/f32-accumulate, and DSD_ASSIGNMENT_MAPPING has no integer MAC builtin at all (only @fmach/@fmachs/@fmacs map to FMADSDOp) -- these shapes fall through to the scalar loop, matching upstream's own declared support surface rather than inventing new CSL builtins. SDK verification for @increment_dsd_offset's elem_type parameter: docker exec into cs_sdk_run was unavailable (daemon was down; once started, the container/image itself is not present in this environment -- it belongs to a separate measurement rig). Verified instead against the official CSL language reference (sdk.cerebras.ai/csl/language/dsds, current docs matching SDK 2.10.x): "elem_type ... must be an ABI-compatible numeric type (u16, i16, u32, i32, f16, or f32)" -- f16 is explicitly listed as valid. Manually verified all 5 dtype combinations against the real lowering pipeline (ad hoc script, not committed): all-f16 -> @fmach, all-f32 -> @fmacs, f32-mul/f16-acc -> @fmachs, f16-mul/f32-acc -> correctly NOT vectorized, all-i16 -> correctly NOT vectorized. Full suite: 381 passed, 12 skipped (unchanged).
Per issue spcl#69 ask 5 (maintainer: "this generalizes DSD detection to ND loops" -- no separate flag needed), remove --disable-mac-vectorization entirely: - spada/cli/compiler.py: drop the click option, the generate_program()/compile_spatial_ir() parameter, and the pass-through call argument. - spada/cli/appliance_compiler.py: same, for compile_spatial_ir_appliance() (this entry point postdates P4 -- it never had the flag to begin with, so this is purely keeping it in sync with compiler.py's shared codegen_options). - spada/lowering/spatial_ir_to_csl.py: drop disable_mac_vectorization from lower_spatial_ir_to_csl() and generate_rectangle(), and the `cslstmt.DISABLE_MAC_VECTORIZATION = True` assignment. - spada/syntax/csl/statements.py: drop the DISABLE_MAC_VECTORIZATION module global; emit_for's gate is now `if not dsd_ops.DISABLE_DSD:` only, with no second condition. Also rewrote the pass's header comment to reflect the current (order- and dtype-generalized, still 2-level-only) scope and record the @increment_dsd_offset f16 verification. - tests/spatial_ir/test_mac_vectorization.py: rewrote test_scalar_fallback_when_disabled to pass disable_dsd=True (was disable_mac_vectorization=True) and reset dsd_ops.DISABLE_DSD (was cslstmt.DISABLE_MAC_VECTORIZATION) in its cleanup -- both are sticky module-level globals, so the reset target had to change along with the flag. Semantic note: this was already true in practice before this commit -- emit_for's old gate was `if not dsd_ops.DISABLE_DSD and not DISABLE_MAC_VECTORIZATION`, so --disable-dsd already disabled MAC vectorization too. What's removed is the ability to disable *only* MAC vectorization while leaving other DSD ops enabled; no prior behavior for --disable-dsd users changes. Verified both CLI entry points still parse correctly (`python -m spada.cli.compiler --help` / `python -m spada.cli.appliance_compiler --help`) with no --disable-mac-vectorization option and no crash. Full suite: 381 passed, 12 skipped (unchanged).
Extends tests/spatial_ir/test_mac_vectorization.py from 6 to 20 tests
to cover the (2) order and (3) dtype generalizations:
- test_mac_loop_vectorized_for_order_and_dtype: positive matrix,
{kl, lk} x {(f32,f32)->@fmacs, (f16,f16)->@fmach,
(f32 mul,f16 acc)->@fmachs} = 6 cases.
- test_no_vectorization_for_unsupported_dtype_combo: {kl, lk} x
{(f16 mul, f32 acc) -- no FMADSDOp branch, (i16, i16) -- no
integer @FMac* builtin} = 4 cases.
- The four pre-existing negative tests (accumulator aliases matrix,
accumulator aliases vector, non-affine index, accumulator differs
from destination) are now parametrized over {kl, lk} = 8 cases,
up from 4, to confirm the aliasing/affine/accumulator guards still
apply correctly regardless of which position declares the row vs.
reduction variable.
_kernel() gained mat_ty/out_ty parameters (defaulting to the original
all-f32 behavior) so stream types can vary with the tested dtype
combination -- receive/send would otherwise reject a dtype mismatch
before MAC vectorization is ever reached.
Armed per art.14 (verified by temporarily breaking the guard under
test and confirming the corresponding new cases go red, then
reverting):
- Aliasing guard disabled -> all 4
test_no_vectorization_when_accumulator_aliases_{matrix,vector}[kl|lk]
cases fail as expected (vectorization no longer suppressed).
- Dtype gate bypassed (falling back to @fmacs unconditionally) -> all
4 test_no_vectorization_for_unsupported_dtype_combo[...] cases fail
as expected.
Both reverted; suite is green again with the real guards in place.
Full suite: 395 passed, 12 skipped (baseline 375 + 20 in this file,
0 regressions).
Leftover line wrap from adding then removing disable_mac_vectorization across the earlier P4-rebase/flag-removal commits; no functional change. Rejoins compile_spatial_ir's signature to match upstream's original formatting so the final diff against eaf8d1c is minimal.
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #69 — first of the two stages proposed there. This covers asks (2), (3), and (5); arbitrary nesting depth (1) and generalizing the affine approach to any DSD operation (4) are left for a second PR, pending the
increment_dsdrelative-vs-absolute offset question in the issue thread.Loop order (2). Loop-variable roles are now detected from the destination index's affine decomposition — the variable appearing as the bare destination index is the row variable, the other is the reduction variable — so both
for (k, l)andfor (l, k)declaration orders vectorize. The positional hardcoding is gone.Dtypes (3). The f32 gate is replaced by a dispatch that mirrors
FMADSDOp._as_cslexactly (sharingdsd_ops._get_base_dtype):@fmach,@fmachs,@fmacsfor the same dtype combinations FMADSDOp models. Integer dtypes are intentionally out of scope —DSD_ASSIGNMENT_MAPPINGcarries no integer MAC builtin. Per the SDK language reference,@increment_dsd_offset'selem_typeaccepts any ABI-compatible numeric type includingf16, so the offset-stepping side generalizes with it.Flag (5).
--disable-mac-vectorizationis removed; the pass is now gated solely by--disable-dsd, as suggested. This changes nothing for existing--disable-dsdusers (the pass was already AND-gated behind it). The negative test that isolated MAC vectorization from other DSD emission was rewritten against--disable-dsd.Tests.
test_mac_vectorization.pygrows 6 → 20: positives across order × dtype, negatives for unsupported dtypes, and the existing aliasing / non-affine / accumulator-mismatch negatives replicated across both loop orders. Full suite: 395 passed / 12 skipped (baseline at eaf8d1c: 375 / 12 — zero regressions). f32 gemv codegen emits a byte-identical vectorized MAC block relative to the pre-change output.🤖 Generated with Claude Code