[dyn-seq] Convert aie.dma_bd sizes/strides/offset/len to SSA operands (DynamicIndexList) by hunhoffe · Pull Request #3306 · Xilinx/mlir-aie

hunhoffe · 2026-07-09T16:57:14Z

Summary

Converts aie.dma_bd from its legacy positional attribute form to the declarative representation used by aiex.npu.dma_memcpy_nd: each transfer parameter is either a compile-time constant carried in an attribute or a runtime SSA operand. Part of the dynamic runtime sequences milestone (#3222).

Three open issues converge on this change:

Dynamic runtime sequences via standalone TXN encoding + EmitC C++ generation #3222 — runtime-valued sizes/strides/offset/len must be able to flow as SSA operands so one compiled sequence can run at many tensor shapes.
Choose dimensions or sizes + strides for user-facing operations #1922 — some ops take dimensions, others take sizes/strides; this converts dma_bd to the sizes strides form dma_memcpy_nd already uses.
dma_task in programming examples #1919 — dma_bd's 157-line hand-written parser had positional ambiguities (len-without-offset parsed len as offset); going declarative deletes the parser and fixes the ambiguity by construction.

What changed

Op surface (`aie.dma_bd`)

sizes/strides: $dimensions (a BDDimLayoutArrayAttr of <size, stride> pairs) becomes $sizes/$strides via custom<DynamicIndexList> (same mechanism as npu.dma_memcpy_nd). Constants land in static_sizes/static_strides; a runtime value becomes a dynamic operand. The clause elides entirely for a plain linear transfer.
offset/len: use the same static-attr-or-operand split. A constant lands in the new $static_offset/$static_len (OptionalAttr<AIEI32Attr>); only a genuine runtime value becomes an Optional<I32> operand. This matches npu.dma_memcpy_nd's static_offsets and means the common static case never materializes an arith.constant. A small custom<DynamicScalar> directive prints a constant as a bare scalar (offset = 0) and a runtime value as %v.
The hand-written DMABDOp::parse/print (~157 lines) is deleted in favor of the declarative assemblyFormat.

Helpers

New accessors: getMixedSizes/Strides(), getFoldedDimensions(), getConstantDimensions(), getConstantOffset/Len(), hasOffset()/hasLen().
buildWithConstants / buildMixed keep every existing int-taking C++ OpBuilder signature source-compatible, dispatching mixed static/dynamic OpFoldResult values.
FoldConstantBDDimList canonicalization folds constant size/stride/offset/len operands back into their static attributes; a verifier guard rejects inconsistent operand/sentinel counts.

Syntax

Old:

aie.dma_bd(%buf : memref<256xi32>, 0, 256, [<size = 16, stride = 16>, <size = 16, stride = 1>])

New — linear (sizes/strides elided):

aie.dma_bd(%buf : memref<256xi32> offset = 0 len = 256)

New — static ND layout:

aie.dma_bd(%buf : memref<256xi32> offset = 0 len = 256 sizes = [16, 16] strides = [16, 1])

New — runtime len as an SSA operand:

aie.dma_bd(%buf : memref<256xi32> offset = 0 len = %len sizes = [%n, 16] strides = [16, 1])

Python wrapper

New dma_bd() in python/dialects/aie.py mirroring npu_dma_memcpy_nd: one interleaved MixedValues list per sizes/strides (ints and Values mixed freely); the split is internal. offset/len accept an int (→ attribute) or a Value (→ operand).
shim_dma_bd (aiex.py) and the IRON Bd/TileDma path (iron/dataflow/tile_dma.py) route through the wrapper.

Closes #1919
Partially addresses #1922 (objectfifo dimensions* → sizes/strides deferred to a follow-up)

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

…dexList) Converts aie.dma_bd from legacy positional attribute form to the declarative DynamicIndexList representation, matching aiex.npu.dma_memcpy_nd: - sizes/strides: Variadic<I64> + DenseI64ArrayAttr static_* + custom<DynamicIndexList> assemblyFormat, enabling runtime values to flow as SSA operands while keeping constants in the static array (no arith overhead for fully-static BDs). - offset/len: Optional<I32> SSA operands (same scalar-operand idiom as #3225's npu write32/push_queue ops). Constant offset/len materialize arith.constant; runtime values enable dynamic-length transfers without recompilation. - Adds arith as a dependent dialect of the AIE dialect (offset/len builders materialize arith.constant). The 157-line hand-written DMABDOp::parse/print is deleted in favor of the declarative assemblyFormat, fixing the #1919 parser ambiguity (positional optional args that silently parsed wrong combinations). New accessors: getMixedSizes/Strides, getFoldedDimensions, getConstantDimensions, getConstantOffset/Len. buildWithConstants funnels all int-taking C++ OpBuilder calls so producers (AIEObjectFifoStatefulTransform, AIELowerMemcpy, etc.) compile unchanged. On the static lowering path, getConstantDimensions folds the mixed list to a BDDimLayoutAttr array (consumer sites: AIEDMATasksToNPU, AIERT, AIETargetXAIEV2) with a clean diagnostic (not a miscompile) for runtime values. The LinearizeContiguousBDTransfer canonicalizer is updated to clear the new operand/static arrays correctly. Byte-identical gate: test/Targets/AIEGenerateXAIE (XAIEV2 register-write goldens) 37/38 pass unchanged; one pre-existing failure unrelated to this change. Closes #1919. Partial #1922 (dma_bd only; objectfifo deferred). Part of #3222. Co-Authored-By: Claude <noreply@anthropic.com>

Mechanical migration of ~312 test .mlir files from the legacy positional aie.dma_bd syntax to the new declarative DynamicIndexList form: Old: aie.dma_bd(%buf : memref<256xi32>, 0, 256, [<size = 16, stride = 1>]) New: aie.dma_bd(%buf : memref<256xi32> offset = %c0_i32 len = %c256_i32 sizes = [16] strides = [1]) Constants for offset/len are materialized as arith.constant in the enclosing region (outside aiex.dma_configure_task bodies, which only allow DMABDOp, UseLockOp, NextBDOp, and EndOp). CHECK lines using the old aie.dma_bd syntax are updated to use the new format with {{.*}} wildcards for SSA operand names. Migration done by utils/migrate_dma_bd_full.py (region-aware balanced-brace scanner). Manual fixups: bad_dma_op.mlir (constant in wrong block position), scratchpad_params (dma_configure_task_for without parens), canonicalize-mem and canonicalize_linear_dma_bd CHECK line updates. Co-Authored-By: Claude <noreply@anthropic.com>

Adds a dma_bd() wrapper in python/dialects/aie.py mirroring npu_dma_memcpy_nd: users pass one interleaved list per sizes/strides (ints and SSA Values mixed freely); the static/dynamic split is internal via _dispatch_mixed_values. offset/len accept ints (materialized as arith.constant i32) or SSA Values. Updates shim_dma_bd (aiex.py) and TileDma.resolve (iron/dataflow/tile_dma.py) to route through the new wrapper using sizes/strides lists instead of the removed dimensions=list(zip(...)) idiom. Co-Authored-By: Claude <noreply@anthropic.com>

Fix 1 (ship-blocker): aiex.py imported dma_bd from the raw generated module (_aie_ops_gen) instead of the aie.py wrapper. After wheel rebuild, shim_dma_bd would crash with TypeError passing plain ints for offset/len. Move dma_bd into the existing from .aie import block. Fix 2: LinearizeContiguousBDTransfer emitted a spurious error diagnostic for valid IR with runtime BD sizes (the new feature), because getFoldedDimensions was called with op.emitError() before confirming all sizes are constants. Pre-check all sizes/strides for constant-foldability; only call getFoldedDimensions (which needs them constant) after that guard passes. Fix 3: LinearizeContiguousBDTransfer injected a len operand when no len was present on a BD with an unranked memref buffer. Add an explicit early-exit for that case (can't safely infer product == full transfer). Fix 4: Padding-exceeds-len check was silently skipped for runtime len operands. Now emits a clear "not yet supported" error when padding is present and len is a non-constant operand, rather than silently omitting the safety check. Fix 5: Nested _as_i32 helper inside dma_bd() (aie.py) was a copy of the module-level helper in aiex.py. Promote to module level in aie.py with a None guard; both copies are now consistently documented as mirrors. Fix 6: Update the dependentDialects comment in AIE.td to accurately explain why dialect-level placement is correct (builder contract requires pre-loading). Fix 7: Replace the opaque emitOpError from getConstantDimensions() in the three static lowering paths (AIEDMATasksToNPU, AIERT, AIETargetXAIEV2) with an explicit up-front runtime-size check emitting a user-facing "not yet supported" message. Add mlir/Dialect/Utils/StaticValueUtils.h include. Fix 8: Change buildWithConstants offset/len parameters from int to int32_t, matching getI32IntegerAttr's parameter type and eliminating the signed-integer type-safety hole for large offset values. New tests: canonicalize_linear_dma_bd gains a section verifying runtime-sized BDs pass --canonicalize silently (no spurious error). nd-dma-pad-runtime-len.mlir pins the Fix 4 "not yet supported" diagnostic. Co-Authored-By: Claude <noreply@anthropic.com>

Black reformats three migration utility scripts in utils/. clang-format reformats two .td files touched by the SSA operand conversion. No functional changes. Co-Authored-By: Claude <noreply@anthropic.com>

AIEOps.td and AIE.td were run through clang-format, which mangled `attr-dict` into `attr - dict` in 26 op assembly formats. mlir-tblgen rejects that ("expected literal, variable, directive, or optional group"), so every build job failed. TableGen files are not clang-format targets; restore both files to keep only the intentional aie.dma_bd DynamicIndexList change. Co-Authored-By: Claude <noreply@anthropic.com>

…ayout Turn static_sizes/static_strides into OptionalAttr and make the printed sizes/strides clause optional, so a plain linear transfer prints as `aie.dma_bd(%buf : memref<...> offset = .. len = ..)` with no trailing `sizes = [] strides = []`. Add a buildMixed helper that dispatches mixed static/dynamic OpFoldResults through dispatchIndexOpFoldResults (leaving the static arrays unset when the layout is empty), a FoldConstantBDDimList canonicalization that folds constant size/stride operands back into the static arrays, and a verifier guard (verifyListOfOperandsOrIntegers + rank match) to avoid the getMixedValues crash on inconsistent operand/sentinel counts. Consolidate the numpy-int-aware _as_i32 helper in aie.py and route the IRON Bd/TileDma path through sizes/strides lists. Co-Authored-By: Claude <noreply@anthropic.com>

…nsfers Update the dma_bd test corpus to the printed form produced by the now-optional static sizes/strides: linear transfers drop the `sizes = [] strides = []` clause entirely. Regenerated CHECK lines from real aie-opt/aie-translate output. The AIEGenerateXAIE XAIEV2 register-write goldens are unchanged byte-for-byte, confirming constants still lower identically. Co-Authored-By: Claude <noreply@anthropic.com>

The earlier mechanical migration (929cf28) over-matched: its balanced-paren rewrite removed arith.constant %c<N> definitions from tests that live near dma_bd patterns but are otherwise unrelated (AIEVec srs/fold, bfp, packet_flow, etc.), leaving those SSA names undefined. Restore the affected files to the correct form so the shift/offset constants they reference are defined again. Co-Authored-By: Claude <noreply@anthropic.com>

With the Bd sizes/strides fields now list-typed, pyright inferred bd_kwargs as dict[str, list] from its sizes=/strides= initializer, then rejected the later int (offset/len) and tuple (packet) assignments. Give bd_kwargs an explicit dict[str, Any] annotation. Co-Authored-By: Claude <noreply@anthropic.com>

github-actions · 2026-07-10T00:10:12Z

Coverage Report

Created: 2026-07-13 16:06

Click here for information about interpreting this report.

Filename	Function Coverage	Line Coverage	Region Coverage	Branch Coverage
Dialect/AIE/IR/AIEDialect.cpp	90.18%	86.18%	87.81%	80.19%
Dialect/AIEX/Transforms/AIEDMATasksToNPU.cpp	95.45%	83.36%	87.88%	78.35%
Dialect/AIEX/Transforms/AIELowerMemcpy.cpp	100.00%	98.63%	92.31%	75.00%
Targets/AIERT.cpp	61.54%	55.75%	44.72%	24.89%
Totals	86.15%	79.89%	67.68%	58.63%

Generated by llvm-cov -- llvm version 18.1.3

… memcpy_nd) Fixes the CI failures on PR #3306. The op previously carried offset/len as plain Optional<I32> operands, forcing an arith.constant for every static dma_bd. That broke two ways: the injected constants stranded across region boundaries in the test migration ("use of undeclared SSA value name"), and constants living inside aie.mem bodies made AIECoreToStandard's op removal erase a mem whose nested constant still had uses (eraseOp assertion). Now offset/len follow the same split sizes/strides and aiex.npu.dma_memcpy_nd already use: a compile-time constant lands in the new static_offset/static_len attribute, and only a genuine runtime value becomes the operand. A small custom<DynamicScalar> directive prints the constant form as a bare scalar (offset = 0 len = 256). getConstantOffset/Len read the attribute first; FoldConstantBDDimList folds a constant offset/len operand back into the attribute; FoldLinearBD sets static_len directly. Test .mlir files are migrated to the attribute form; four npu-xrt tests with pre-existing cross-region stranded constants are regenerated. The AIEGenerateXAIE byte-identical golden set is unchanged (constants lower identically), and the previously-failing npu-xrt tests pass on hardware. Co-Authored-By: Claude <noreply@anthropic.com>

The matrix_multiplication_using_cascade tests deduplicated their per-core arith.constant into a single aie.mem region during the earlier operand-form migration, stranding the memref.store uses in the sibling aie.core regions ("operand does not dominate this use"). Regenerate the four files from the positional source so each region keeps its own constants and the dma_bds use the attribute form. Also correct the now-inaccurate dependent-dialect comment (offset/len no longer materialize arith.constant) and note that the packet zero-length guard in AIERT only covers compile-time-constant lengths. Co-Authored-By: Claude <noreply@anthropic.com>

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

hunhoffe

there are some whitespace changes that aren't fully necessary, but it seems more trouble than they add to go through and remove all of them.

- dma_bd() Python wrapper takes transfer_len (was len) to avoid shadowing the builtin and match shim_dma_bd; still lowers to the op's len operand. - buildWithConstants uses getI32IntegerAttr for offset/len to match AIEI32Attr, keeping the cast sign-exact for negative offsets. - Distinguish _split_i32_scalar from _as_i32 in its docstring (attribute vs materialized arith.constant). Co-Authored-By: Claude <noreply@anthropic.com>

# Conflicts: # test/Passes/objectfifo-liveness/sufficient.mlir # test/Passes/objectfifo-liveness/under_buffered.mlir

andrej

Looks good!

Tersed up three comments but stopped there. I think AI sometimes emits too many and too verbose comments, and it slows down my flow when reading the code. I might have to adapt to this but, to me, a comment has historically meant "something atypical/complex that needs explanation is happening here", so I have given them a lot of weight when reading code. I think now it is often more "the AI was prompted to redo/change this section of code so felt the need to explain itself."

This is obviously majorly nitpicky just trying to explain where I'm coming from.

Good progress on the dynamic runtime sequence!

…n form The self-review commit renamed the dma_bd() wrapper's len kwarg to transfer_len (avoids shadowing the builtin, matches shim_dma_bd) and made a constant len land in the static_len attribute rather than an arith.constant operand. This test still passed len= and CHECK'd the operand form, so it hit "got multiple values for keyword argument 'len'". Route through transfer_len and expect the bare `len = 2`. Co-Authored-By: Claude <noreply@anthropic.com>

Apply Andrej's review suggestions (AIE.td arith dependency, dma_bd offset/len docs, buildWithConstants) and give the sibling comments the same concise treatment: drop the internal cross-references and upstream-op name-drops, keep the load-bearing "why". Co-Authored-By: Claude <noreply@anthropic.com>

hunhoffe · 2026-07-13T15:47:37Z

@andrej Thanks for the review! I agree -- you caught me being lazy about forcing concise comments, but I tried to do some clean up now.

Prove milestone #3222's correctness spine: a runtime_sequence compiled to a C++ TXN builder (aie-translate --aie-npu-to-cpp) produces a byte-identical TXN word stream to the production binary emitter (--aie-npu-to-binary) when invoked with matching constants, and runtime scalar args flow in as function parameters. Each test lowers a static and a dynamic sequence to terminal npu ops and does a three-way byte-identical compare: binary golden vs generate_txn_<static>() vs generate_txn_<dynamic>(N). The split localizes drift to EmitC codegen vs runtime-arg substitution. Coverage: - Both DMA lowering paths, which converge on the same terminal ops but nothing else in-tree exercises end to end: dma_memcpy_nd (memcpy_nd*.mlir) and the DMA-task path (dma_task*.mlir; dma_configure_task_for/start/await, SSA bd_id from #3225, the only path reaching the sync op). - Both device generations (npu2 + npu1 variants), since device info is baked into the header words. - Every op the C++ TXN target supports: write32, maskwrite32, blockwrite, address_patch, sync. - Two sizes on the memcpy path (N=4096 and N=8192) to exercise the add-a-size recipe and guard the runtime-arg path at more than one value. The DMA-task sequences carry aie.dma_bd in the declarative sizes/strides/offset/ len form (#3306); the C++ builder name is generate_txn_<device>_<seq>, so the comparator is parameterized against generate_txn_main_<seq>. compare_main.cpp is fully -D parameterized so one comparator serves every path, device, and size; README documents the add-a-size/add-a-pattern recipe. Co-Authored-By: Claude <noreply@anthropic.com>

Integrates PR #3308 (dynamic lock acquisition for ObjectFIFO): - All test files: use_lock uses new SSA i32 value operand form (arith.constant + %c) - All test files: dma_bd retains this PR's SSA operand format (offset=/len=/sizes=/strides=) - bad_bd_assignments.mlir: includes #3308's new nextBdId overflow test - dynamic_cyclostatic_* test files removed (deleted upstream by #3308) - New dma_channel_pinning/ tests and dynamic_runtime_lock_* tests from #3308 Co-Authored-By: Claude <noreply@anthropic.com>

jgmelber requested a review from Copilot July 9, 2026 17:31

Copilot started reviewing on behalf of jgmelber July 9, 2026 17:33 View session

Copilot AI reviewed Jul 9, 2026

hunhoffe commented Jul 9, 2026

View reviewed changes

Comment thread include/aie/Dialect/AIE/IR/AIE.td

Comment thread python/dialects/aie.py

Comment thread python/iron/dataflow/tile_dma.py Outdated

Comment thread test/create-locks/test_lock6.mlir Outdated

hunhoffe and others added 13 commits July 9, 2026 14:19

Apply black and clang-format to PR-touched files

e4bffbe

Black reformats three migration utility scripts in utils/. clang-format reformats two .td files touched by the SSA operand conversion. No functional changes. Co-Authored-By: Claude <noreply@anthropic.com>

Delete utils/migrate_dma_bd.py

3e04220

Delete utils/migrate_dma_bd_full.py

6455bf8

Delete utils/migrate_run.py

e222fa6

Delete utils/migrate_validate.sh

e77b6d1

hunhoffe force-pushed the dma-bd-ssa-operands branch from dc79e44 to c35a102 Compare July 9, 2026 23:46

Merge branch 'main' into dma-bd-ssa-operands

3d2fe5b

hunhoffe changed the title ~~[dyn-seq] Convert aie.dma_bd sizes/strides to SSA operands (DynamicIndexList)~~ [dyn-seq] Convert aie.dma_bd sizes/strides/offset/len to SSA operands (DynamicIndexList) Jul 9, 2026

hunhoffe added this to the IRON 1.4 milestone Jul 9, 2026

hunhoffe and others added 2 commits July 10, 2026 16:23

Merge branch 'main' into dma-bd-ssa-operands

399ffdc

hunhoffe marked this pull request as ready for review July 10, 2026 23:01

hunhoffe requested review from abisca, denolf, jgmelber and stephenneuendorffer as code owners July 10, 2026 23:01

hunhoffe requested review from andrej, erwei-xilinx, fifield, jackl-xilinx and pvasireddy-amd as code owners July 10, 2026 23:01

hunhoffe and others added 3 commits July 10, 2026 17:21

Update lib/Targets/AIERT.cpp

ea2ac99

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Merge branch 'main' into dma-bd-ssa-operands

1b0be15

hunhoffe commented Jul 11, 2026

View reviewed changes

Comment thread python/dialects/aie.py

hunhoffe and others added 2 commits July 13, 2026 08:26

Merge remote-tracking branch 'origin/main' into dma-bd-ssa-operands

e9456b4

# Conflicts: # test/Passes/objectfifo-liveness/sufficient.mlir # test/Passes/objectfifo-liveness/under_buffered.mlir

andrej approved these changes Jul 13, 2026

View reviewed changes

Comment thread include/aie/Dialect/AIE/IR/AIE.td Outdated

Comment thread include/aie/Dialect/AIE/IR/AIEOps.td Outdated

Comment thread include/aie/Dialect/AIE/IR/AIEOps.td Outdated

hunhoffe and others added 2 commits July 13, 2026 09:34

hunhoffe enabled auto-merge July 13, 2026 15:47

hunhoffe disabled auto-merge July 13, 2026 16:23

hunhoffe and others added 2 commits July 13, 2026 13:32

Merge branch 'main' into dma-bd-ssa-operands

b3d6605

hunhoffe enabled auto-merge July 13, 2026 19:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[dyn-seq] Convert aie.dma_bd sizes/strides/offset/len to SSA operands (DynamicIndexList)#3306

[dyn-seq] Convert aie.dma_bd sizes/strides/offset/len to SSA operands (DynamicIndexList)#3306
hunhoffe wants to merge 26 commits into
mainfrom
dma-bd-ssa-operands

hunhoffe commented Jul 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 10, 2026 •

edited

Loading

Uh oh!

hunhoffe left a comment

Uh oh!

Uh oh!

andrej left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hunhoffe commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

hunhoffe commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Op surface (aie.dma_bd)

Helpers

Syntax

Python wrapper

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Created: 2026-07-13 16:06

Generated by llvm-cov -- llvm version 18.1.3

Uh oh!

hunhoffe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrej left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hunhoffe commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hunhoffe commented Jul 9, 2026 •

edited

Loading

Op surface (`aie.dma_bd`)

github-actions Bot commented Jul 10, 2026 •

edited

Loading