Skip to content

DRAFT: flm/layer: let the compiler assign the decode layer's lock ids - #242

Draft
andrej wants to merge 3 commits into
amd:develfrom
andrej:flm-decode-lock-move
Draft

andrej wants to merge 3 commits into
amd:develfrom
andrej:flm-decode-lock-move

Conversation

@andrej

@andrej andrej commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

The decode layer's design pinned every lock id by hand. Its tables left gaps
for the ids an ObjectFifo takes, so a reader had to know which ids were free
to read them.

This PR removes every hand-written lock id. It pairs with
Xilinx/mlir-aie#3912, which gives up the locks the kernels used to take.

What the compiler can and cannot assign

A flm_gemma4_decode_* kernel takes a lock id as a compile-time constant,
through a -DFLM_GEMMA4_DECODE_<STEM>_<NAME> flag. The id must exist before
the kernel object compiles, which is before --aie-assign-lock-ids runs. The
design therefore still pins every lock a kernel names.

It now pins them from 0 up, in the order it declares them, which is the id
the pass would pick anyway. _lock_ids builds each table from that order, so
no table holds a number any more. Two ids come from a neighbouring tile's
table, because the kernel reaches across the tile boundary for them.

Every other lock takes lock_id=None and the pass picks it: both projection
gather memtiles, the projection weight and x memtiles, the attention memtile,
the per-layer-input merge tile, and the locks the kernels give up in
mlir-aie#3912. The design emits those acquires and releases from its Worker
bodies, against the aie.lock SSA value.

The runtime sequence sets each RTP barrier with Lock.set, so it no longer
computes 0x1F000 + 16 * id.

105 of the design's 259 locks now take a compiler-assigned id, up from 53 of
261. The rest are named by a kernel that takes them inside a loop or an
RTP-dependent branch.

Merge order

This PR needs a mlir-aie that contains Xilinx/mlir-aie#3912. That has to
merge first and ship in a wheel; this PR then bumps mlir_aie in
requirements.txt to that version in the same change. It stays a draft until
then. devel keeps working in the meantime, because it pins an exact
mlir_aie version and does not pick the new kernels up on its own.

Validation

pytest iron/operators/flm/layer/test.py on an NPU2 (Strix) gives 38 passed,
1 skipped, against a local mlir-aie checkout carrying #3912. That covers both
geometries, all four layer types, and the context lengths that put a
sliding-window layer before, at and after its ring wrap. The skip is the
captured dispatch case, which needs FLM_LAYER_CASES.

The suite does catch a wrong lock id: it shows up as ERT_CMD_STATE_TIMEOUT
or as a relative-L2 assert. Removing all the ids at once, before the kernels
gave theirs up, deadlocked exactly that way.

test_matches_reference also runs _check_rtps, so each RTP buffer still
lands at the address the engine writes.

The design pinned every lock id by hand, in tables that left gaps for the
lock ids an ObjectFifo takes. A reader had to know which ids were free to
read the tables.

A kernel takes a lock id as a compile-time constant, so the design must still
pin every lock a kernel names. It now pins them from 0 up, which is the id
the lowering would pick anyway, and `_lock_ids` derives each table from the
order the design declares its locks. No lock id is written by hand.

Every other lock takes `lock_id=None`, so `--aie-assign-lock-ids` picks it:
the projection and attention memtiles, the per-layer-input merge tile, and
the locks that the kernels gave up in Xilinx/mlir-aie#3912. The design emits
those acquires and releases from its Worker bodies instead.

The runtime sequence sets each RTP barrier through `Lock.set`, so it no
longer writes `0x1F000 + 16 * id` by hand.

105 of the design's 259 locks now take a compiler-assigned id, up from 53 of
261.
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

CI performance trends

7b92dd7 (2026-10-16T18:28:05Z)

No benchmark moved past the reporting threshold.

⚠️ No results from: Krackan - Operators (failure), Krackan - Applications (failure), Phoenix - Operators (failure). This comment covers the remaining suites only.

This branch has not been deployed

No deployments
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