Replies: 6 comments 1 reply
-
|
@hunhoffe this looks like a great plan. It has evolved much since my initial naive stab... |
Beta Was this translation helpful? Give feedback.
-
|
The analysis on AIRRt makes sense today. The goal of the airrt-to-npu pass in mlir-air is essentially to satisfy today's mlir-aie runtime sequence being fully static. The pass attempts to digest every SSA value into constants via (tiling, for NPU BD optimization) unrolling and canonicalization. |
Beta Was this translation helpful? Give feedback.
-
|
@jgmelber I forgot some things so now it's even more involved, but hopefully I've articulated the pitfalls well |
Beta Was this translation helpful? Give feedback.
-
|
I'm just checking that I'm understanding this correctly, but the idea is to turn If that is so... I like this idea, and I like it a whole lot. I've been wanting something like this for a while. The ambition is a bit staggering, though. It sounds like this may eventually become one unified, parametric, scheduling model, where the existing static lowering is mostly just a specialization. That actually brings me to my first comment/concern: overhead. The current static method doesn't have to think; it just has to do. But if it is built on top of more general machinery, cycles can get burned needlessly. As long as "byte-identical" means "literally output the same thing as today without a control program/middleman recreating it", then you have that covered. But for the dynamic side, I'm not sure it is going to end up being as simple as SSA use-liveness. I think Phase 0 is already showing that there is a bit more to it than that. Single-digit microseconds and nobody will know the difference, but if reconfiguration starts taking dozens, or hundreds, then it becomes a "robbing Peter to pay Paul" problem (at least for one of the workloads I have in mind). On GPU, a UAV barrier for memory access reconfigure is almost always sub 10 microseconds as far as I've measured on Strix (usually more like 2-6). That would be nice to see with this in terms of switch-over times. But, given the CPU overhead, I think anything under ~75 us for a full invocation would be fine-ish (but more akin to a small GPU dispatch boundary, during which work is getting done). Also, from a user-program perspective, I think actually making use of this in most cases is probably going to require XRT fences, because polling will just tank performance. We have those on Windows, but we'll have to bug Soren for a fresher SDK build to pick up the usability fixes he made in that area a few months ago. I think there is probably a ton of latency-hiding potential to be had with that as well, depending how clever the control layer gets, because you can async NPU work from CPU setup. Second, I think it is probably going to have to be documented from the start exactly what this can and cannot do. Obviously workers, tile placements, the lock/ObjectFIFO topology, and core kernels are not going to be realistically touchable by something like this. But that still leaves offsets, strides, transfer extents, repeat/loop behavior, and conditional behavior as much more flexible targets than they were before. I think you were implicitly saying all that, but I had to really think through the implications before it became truly clear. I mean, now it is clear from the start. I just didn't quite grasp the shape of it at first. Third, after thinking this over, I'm wondering whether now would be a good time to implement a canonical form of finish-on-TLAST? I don't mean that it needs to become another prerequisite for this project, but the dynamic specialization could probably make a very real early use of it: fixed hardware topology, but runtime-controlled transfer/termination behavior. Maybe that could extend IO flexibility enough to solve some problems? Sorry, I spent so much time restating things you've already said. I'm just reasoning through this. I haven't spent very much time thinking about this layer or the project yet. |
Beta Was this translation helpful? Give feedback.
-
Small correction to the Phase 0a TCT criterionThe TCT balance check is implemented (#3254, split out of #3226 and verified on Strix). While building it I found the direction of the failure mode in 0a needs a flip. The plan says:
... but that's actually legal. Checking against the lowering: a The real bug is the dual — over-consumption: an await on a path where the matching token may not have been produced (the sync drains an empty FIFO and blocks forever). E.g. a token produced in only one So the corrected criterion:
Over-production is legal; only paths that can drain an empty FIFO are rejected, with a clear diagnostic. The rest of 0a stands. |
Beta Was this translation helpful? Give feedback.
-
|
@thomthehound Thanks for taking a look! Yes: Initial Goal: the first real goal is "compile a GEMM once, run it at many matrix sizes" — runtime On overhead — The static/constant-bound case stays on the existing flat-binary path (constant-bound On "more than SSA liveness" — agreed. BD-ID allocation needed region-aware liveness (plain On XRT fences — agreed, great point. Might be follow on work to think more deeply about this, I haven't worked through the full implications of this yet. On finish-on-TLAST — I want to understand your use case before I answer well. I haven't worked through the implications yet either, so rather than hand-wave: can you sketch the workload shape you have in mind? Thanks for participating in this discussion! |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Let an
aie.runtime_sequencebe compiled once and run at many problem sizes. Instead of baking fixed sizes into a static TXN binary, emit the control program as dependency-free C++ that assembles the TXN binary at run time from scalar arguments. One xclbin serves many shapes (e.g. a GEMM with runtimeM/K/N, or a passthrough with runtime length).This is a fresh, incrementally-landable rebuild superseding branch
dynamic-runtime-sequences-ssa(PR #3168), salvaging its good parts (TXN encoder, BD-word encoder, AIEX→EmitC conversion) as clean, individually reviewable PRs.Goals
static_assert-pinned to the existing binary emitter so the two can't drift.generate_txn_sequence(...)function taking runtime scalars as arguments.Non-goals
Legalization strategy: constrain the dynamic domain
airrt's mature legalization is constant-dependent (it inspects concrete sizes to pick tiling factors and unroll loops) so it can't run on runtime values. Analysis shows this isn't a blocker: the dims that are genuinely runtime-valued already stay within hardware limits, while patterns that need legalization come from static structure and can be legalized at compile time as today.
So: static structure is legalized at compile time (existing lowering; airrt port tracked separately); runtime values are allowed only on dims within hardware bounds (per-dim wrap ≤ {64,1024,1024,1024} after scaling, stride ≤ 2²⁰, ≤ 4 dims, inner alignment to address-gen granularity); and the path emits a compile-time warning when a bound can't be statically verified, plus a runtime guard where cheap.
Correctness criteria ("done")
aie-translatecan't substitute SSA i32 args at translate time.)static_assert-pinned between header and binary emitter.PR sequence
Each PR is independently reviewable and landable, ordered to keep the tree green. Grouped into three phases: Phase 0 makes the existing static infrastructure correct under control flow (prerequisites that the prior branch skipped — its allocators silently mis-handle
scf); Phase 1 builds the TXN/EmitC foundation; Phase 2 adds the dynamic capability and control flow on top.Phase 0 — Correctness prerequisites (control-flow-safe static infrastructure)
0a. Control-flow-aware BD-ID / lock allocation via SSA liveness. Today three allocators(
AIEAssignRuntimeSequenceBDIDs,AIEAssignLockIDs,AIEAssignBufferDescriptorIDs) walk the sequence in program order and assume straight-line code.AIEAssignRuntimeSequenceBDIDs.cpp:140says so outright: "if in the future we support branching/jumping… a proper liveness analysis will become necessary." Underscf.for/scf.ifa BD or lock whose live-range crosses a loop back-edge or differs betweenifarms can be assigned an overlapping ID with no diagnostic → silent data corruption. (The existing dynamic GEMM only works because its ping-pong keeps every live-range inside one iteration — unverified luck, not correctness.)dma_configure_taskdefines anIndexresult;dma_start_task/dma_await_task/dma_free_taskconsume it. So BD-ID liveness is SSA value liveness, directly computable by upstreammlir/Analysis/Liveness.h(region/CFG-aware, soscfworks for free). The current allocators are an ad-hoc program-order approximation of whatmlir::Livenessalready computes correctly.BdIdGenerator(the pool/free-list) is already shared between the core and runtime-sequence BD allocators; only the traversal differs. Factor a single CF-aware liveness/traversal layer over the existing generator and use it from all three sites. (Note: mlir-air has no reusable liveness to borrow — it unrolls everything to constants — so the reuse target is upstream MLIR, not air.)repeat_count(the token bit is on the push command,AIEDMATasksToNPU.cpp:107); adma_await_tasklowers to onenpu.syncthat waits on a channel region, with no token-count field. So a loop that pushes N times but awaits once outside the loop silently under-waits and reads incomplete data — a control-flow-dependent imbalance that program-order reasoning cannot catch. The liveness layer must therefore verify push/await (token produce/consume) balance across the CFG, not only BD-ID/lock live-ranges.scf.for,scf.if, and nested cases; a live-range that genuinely exceeds the hardware pool is rejected with a clear diagnostic (never a silent overlap); a push/await token imbalance across control flow is rejected with a clear diagnostic (never a silent under-wait); existing static tests unchanged.Phase 1 — TXN / EmitC foundation
txn_init,txn_prepend_header,txn_append_*(write32, maskwrite32, sync, blockwrite, address_patch, loadpdi, preempt). RefactorAIETargetNPU.cpponto it;static_assertall shared opcodes. Existing TXN tests unchanged (proves byte-neutral). [dyn-seq P1.1] Extract TXN instruction encoding into a standalone header #3224write32/maskwrite32/sync/address_patch/rtp_writetake SSA operands; attribute forms removed; constant operands fold identically; all users migrated; breaking-change note. [dyn-seq P1.2] Carry npu scalar op fields as SSA operands #3225 [dyn-seq] Convert aie.dma_bd sizes/strides/offset/len to SSA operands (DynamicIndexList) #3306get_globalfeeding blockwrite. Generated C++ includes the PR 1 header. Newaie-translatetarget. Unsupported opsemitOpError(never silent). FileCheck per op. [dyn-seq P1.3] AIEX→EmitC conversion + C++ TXN target #3256rtp_write; documented recipe to add a size. [dyn-seq P1.4] Static-vs-dynamic TXN equivalence harness #3289Phase 2 — Dynamic capability + control flow
repeat_count,buffer_lengthwith correct masks/shifts, size-gated strides, element-width/addr-gen rescaling. Shared word layout → byte-identical for constants; only runtime fields becomewrite32overrides. Compile-time warning + runtime guard on bounds. Restrictions enforced with diagnostics (shim NOC only, innermost stride == 1, all-or-nothing dynamic sizes/strides,pad_dimensionsincompatible). A runtime-valuedmemref.subview(the memref must currently trace to a block arg through static subview offsets/sizes/strides,AIEDmaToNpu.cpp:621-626) is rejected with a clean diagnostic, not a crash. FileCheck for valid + invalid cases. [dyn-seq P5] Dynamic BD-word encoder for dma_memcpy_nd #3293repeat_count+ blockwrite-fold interaction. Runtime outer-dim repeat_count; defined, tested behavior when dynamic sizes prevent blockwrite folding (larger per-registerwrite32stream). (Open: may fold into PR 5.)scf.for/scf.ifwith constant bounds in a runtime sequence are fully unrolled to a flat op list and lowered down the existing binary TXN path. The binary path rejects non-constant-bound control flow with a clean diagnostic (never crashes, never silently drops a loop). Establishes the oracle the next PR relies on: a constant-bound sequence lowered two ways (unroll→binary vs. scf→EmitC fed the same constants) must produce byte-identical TXN, checked via the PR 4 harness. FileCheck for the unrolled lowering and for the reject diagnostic. Prior art (mlir-air, copy the recipe down):unrollSCFFors/unrollAffineFors(AIRRtToNpuPass.cpp:2147/:2106) — constant-boundloopUnrollFull+ post-unroll walk that renumbers per-op metadata (the BD-ID/symbol analogue), andscf.forall→scf.fornormalization viaforallToForLoop(:1434) which must precede unrolling. Required acceptance criterion (known silent-failure trap): after unrolling, foldaffine.apply/arithops whose operands became constants — induction vars unroll to constants butaffine.apply(constant)does not auto-fold, and downstream constant reads then silently fall back to 0 (seeAIRRtToNpuPass.cpp:1456-1465). Test that an unrolled loop carrying an induction-derived offset lowers to the correct (non-zero) value.loopUnrollFullonly).scf.for(runtime bounds),scf.if(runtime condition), and arbitrary nesting (loop-in-loop, if-in-loop) survive through AIEX→EmitC into real C++for/ifvia upstreamconvert-scf-to-emitc. This is the integration point of SSA-operand ops + EmitC + the dynamic BD encoder, so it is where all three are proven together. Acceptance criteria, each tested:scf.yieldvalues, externally-defined values used inside, in-loop values used after. Non-arith.constantexternal operands are rejected with a diagnostic (today'scloneExternalConstantslimitation) — test both accepted and rejected.dma_memcpy_ndwith runtime sizes inside anscf.forwith runtime trip count) — the stacked target case (tiled dynamic GEMM).M/K/N, including a runtime-loop-tiled variant) compile once, run at multiple shapes on NPU1+NPU2, results verified; driven through the high-level flow (xclbin + generated C++ TXN, no static insts); lit gated on hardware.columnNum = rowNum = 1(AIEDMATasksToNPU.cpp:177-178), so a multi-column design emits N separate (correct) 1×1 syncs rather than one wide sync — functionally correct, not collapsed. Add a dynamic design spanning >1 column to the hardware matrix to confirm correctness (the dynamic path has no multi-column coverage today); collapsing into wide syncs is an optional later optimization, not required here.arg_idx = -1("append the trace buffer after the last tensor") computes its DDR offset from the static memref shape (memrefType.getNumElements() * elemBytes,AIEInsertTraceFlows.cpp:165-166) and a compile-timebufferSize(:150). With a runtime-sized output buffer (e.g.C[M×N]) that offset is wrong and the trace silently lands at the wrong DDR location, corrupting trace or output. (The branch's on-hardware trace verification only ever exercised static-size passthrough.) Fix: support trace via an explicit, separate trace argument (its own buffer, not appended) whose offset does not depend on a runtime-sized tensor; and rejectarg_idx = -1+ runtime-sized buffers with a clear diagnostic rather than miscompiling. FileCheck for the rejection; hardware check of an explicit trace buffer on a dynamic design.Coverage-gap PRs (parallelizable after PR 3, one scoped PR each, FileCheck + equivalence): extend EmitC to
push_queue,preempt,load_pdi,create_scratchpad/update_from_scratchpad,control_packet,dma_wait,writebd. (Open: confirm which are needed vs. deferrable for target workloads.)Related effort (separate milestone)
Port airrt's static BD legalization into mlir-aie —
tileIllegalWrapDim,canonicalizeWrapAndStrideList, and alignment helpers (getDmaInnerElementAlignment,findLargestAlignedFactor,isContiguousRowMajorOrRepeated). Mature, broadly useful for lowering static patterns to legal BDs; benefits the whole compiler independent of this work. Because mlir-aie is strictly below mlir-air, it's a one-way port down (mlir-air must never become a dependency). This milestone depends only on the hardware bounds those helpers encode, not the helpers themselves.Beta Was this translation helpful? Give feedback.
All reactions