Skip to content

[AIR] getRefeedCount: explicit per-emission override wins for any value - #1758

Merged
erwei-xilinx merged 4 commits into
Xilinx:mainfrom
erwei-xilinx:air-refeed-explicit-override
Jul 29, 2026
Merged

[AIR] getRefeedCount: explicit per-emission override wins for any value#1758
erwei-xilinx merged 4 commits into
Xilinx:mainfrom
erwei-xilinx:air-refeed-explicit-override

Conversation

@erwei-xilinx

@erwei-xilinx erwei-xilinx commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • air.refeed_count on a channel put/get is lowered to AIE lock acquire/release values (and the write-lock init). air::getRefeedCount(ChannelInterface) previously honored a per-emission override only when its value was > 1, falling through to the channel-level declaration otherwise.
  • This meant an explicit per-emission value of 1 could not locally disable a shared channel's re-feed default. It also meant a producer had no way to express a count-free re-broadcast as value-1 per emission — so a refeed_count beyond the target's max lock value (AIE-ML locks are 7-bit, max +63) would lower to an unsatisfiable AcquireGreaterEqual and deadlock, with no local escape hatch.
  • Fix: an explicit per-emission air.refeed_count now wins for any value; only when the put/get carries no explicit attribute do we fall back to the channel declaration. The override is gated on an IntegerAttr so a malformed/wrong-typed attribute falls through to the channel declaration rather than silently disabling the re-feed.

Scope / what this does NOT do

  • This makes the per-emission value authoritative and gives a producer a local escape hatch (express the re-broadcast as value-1 per emission to keep every lock value in range). It does not add hardware-lock-max validation: a refeed_count in [64, INT32_MAX] still verifies and would still exceed a 7-bit lock. Bounding the count to the target's actual max lock value (or falling back to a multi-buffer scheme) is left as follow-up.

Validation

  • air.refeed_count was validated ([1, INT32_MAX], integer type) only on the channel declaration. Now that a per-emission value is authoritative, the same check is applied to the put/get carrier: the range/type check is extracted into verifyRefeedCountAttr and shared by the ChannelOp, ChannelPutOp, and ChannelGetOp verifiers. A malformed per-op value now errors at verification instead of being silently normalized to 1 in the reader.

Compatibility

Behavior change is limited to ops that explicitly set air.refeed_count == 1: they now return 1 instead of the channel-level default. Ops with no explicit attribute, or an explicit value > 1, are unchanged. No pass emits an explicit per-op value of 1, so auto-generated IR is unaffected.

Test plan

  • ninja check-air-mlir — AIR dialect + AIRToAIE suites pass (141/141 locally).
  • A put/get with explicit air.refeed_count = 1 on a channel whose declaration sets a higher default lowers to value-1 locks (override honored), not the channel default — folded into air_channel_producer_refeed.mlir with the file's triple CHECK-NOT idiom (init / AcquireGreaterEqual / Release) on a unique count.
  • Per-op invalid-IR cases (put zero / huge / non-integer, get negative) rejected by the new put/get verifier, plus a valid value-1 override — added to air_channel_invalid.mlir.

air.refeed_count on a channel put/get is lowered to AIE lock acquire/release
values (and the write-lock init). The per-emission override previously only won
when its value was > 1, so an explicit value of 1 could not locally disable a
shared channel's re-feed default -- and any refeed_count exceeding the target's
max lock value (AIE-ML locks are 7-bit, max +63) silently produced an
unsatisfiable AcquireGreaterEqual and deadlocked, because the count sizes a
single hardware semaphore rather than being expressed count-free.

Make an explicit per-emission attribute win for ANY value (hasAttr check), so a
shared channel's re-feed default can be locally disabled (e.g. a value-1
re-broadcast that must not inherit the channel's count). Only fall back to the
channel declaration when the put/get carries no explicit attribute at all.

Behavior change is limited to ops that explicitly set air.refeed_count == 1:
they now return 1 instead of the channel-level default. Ops with no explicit
attribute, or an explicit value > 1, are unchanged.
Copilot AI review requested due to automatic review settings July 29, 2026 04:04
@erwei-xilinx
erwei-xilinx requested a review from fifield as a code owner July 29, 2026 04:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates AIR’s getRefeedCount(ChannelInterface) resolution so that an explicit per-emission air.refeed_count attribute on a channel.put/get takes precedence for any value (including 1), instead of only when > 1. This supports locally disabling a shared channel’s refeed default during lowering to AIE locks.

Changes:

  • Change override detection from “returned count > 1” to “attribute is explicitly present on the emission op”.
  • Keep channel-declaration fallback only when the emission carries no explicit air.refeed_count.
Comments suppressed due to low confidence (1)

mlir/lib/Util/Util.cpp:714

  • Using hasAttr() treats any presence of air.refeed_count (including the wrong attribute type) as an explicit override and forces the result to 1 via getRefeedCount(Operation*)'s fallback. Previously a malformed/non-integer per-emission attribute would fall through to the channel declaration; with this change it can silently disable a channel-level refeed default. Consider requiring an IntegerAttr here (and warning/falling back to the channel decl if the attribute exists but is not an integer).
  if (op.getOperation()->hasAttr(air::attrs::RefeedCount))
    return getRefeedCount(op.getOperation());
  return getRefeedCount(getChannelDeclarationThroughSymbol(op).getOperation());

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mlir/lib/Util/Util.cpp Outdated
erwei-xilinx and others added 3 commits July 28, 2026 21:11
A channel declared refeed_count=4 with a producer put that sets refeed_count=1
must lower to the value-1 override, not the channel's 4. Guards against a
regression that only honors per-op overrides > 1.
Address review feedback: gate the per-emission override on an IntegerAttr rather
than hasAttr, so a malformed/wrong-typed air.refeed_count attribute falls through
to the channel-level declaration instead of silently disabling the re-feed
(getRefeedCount(Operation*) would treat a non-integer attr as 1).
The per-emission air.refeed_count override on a channel put/get is now an
authoritative carrier (getRefeedCount honors it for any value), but only
ChannelOp::verify() validated the count -- a malformed per-op value was silently
normalized to 1 in the reader instead of erroring like a channel-level one.

- Extract the [1, INT32_MAX] / integer-type check into verifyRefeedCountAttr and
  call it from ChannelOp, ChannelPutOp, and ChannelGetOp verifiers, so the two
  authoritative carriers validate identically.
- Add per-op invalid-IR tests (put zero / huge / non-integer, get negative) plus
  a valid value-1 override, mirroring the existing channel-level cases.
- Fold the standalone override lit test into air_channel_producer_refeed.mlir as
  a new section using a unique count (5) with the file's triple CHECK-NOT idiom
  (init / AcquireGreaterEqual / Release), strengthening the single-CHECK-NOT
  original and removing duplicated launch/segment/herd boilerplate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@erwei-xilinx
erwei-xilinx added this pull request to the merge queue Jul 29, 2026
Merged via the queue into Xilinx:main with commit 5c49b46 Jul 29, 2026
27 checks passed
@erwei-xilinx
erwei-xilinx deleted the air-refeed-explicit-override branch July 29, 2026 23:54
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.

2 participants