Skip to content

Remove redundant depth-1 exception from objectFifo lowering#2986

Draft
abisca wants to merge 5 commits intomainfrom
objfifo-depth-one
Draft

Remove redundant depth-1 exception from objectFifo lowering#2986
abisca wants to merge 5 commits intomainfrom
objfifo-depth-one

Conversation

@abisca
Copy link
Copy Markdown
Collaborator

@abisca abisca commented Mar 18, 2026

The findObjectFifoSize() function in AIEObjectFifoStatefulTransform had a special case that returned 1 buffer when both the declared scalar depth and the max acquire count were 1, suppressing the usual maxAcquire + 1 ping-pong increment. This exception was redundant because the array-depth form (e.g. [1, 1]) already provides a way to explicitly enforce per-endpoint depths. Remove the exception so that a scalar depth-1 objectFifo consistently lowers to two buffers like any other scalar depth, and document that [1, 1] is the correct way to enforce a single buffer.

Changes:

  • Remove the maxAcquire == 1 && objFifo.size() == 1 guard from findObjectFifoSize() in AIEObjectFifoStatefulTransform.cpp
  • Update depth_one_objectfifo_test.mlir to use [1, 1] array depth so the test continues to verify single-buffer lowering via the explicit array-depth mechanism
  • Add depth_one_scalar_test.mlir: scalar depth 1 now lowers to 2 buffers (ping-pong)
  • Add depth_one_array_test.mlir: array depth [1, 1] still lowers to 1 buffer, confirming the enforcement mechanism works
  • Add depth_two_scalar_test.mlir: scalar depth 2 with max acquire 1 lowers to 2 buffers (not 3), confirming buffer count is driven by maxAcquire rather than the declared size
  • Update 01_single_double_buffer/ example designs to use depth 2 (ping-pong) and add a note that [1, 1] enforces a single buffer

abisca and others added 2 commits March 17, 2026 03:11
The `findObjectFifoSize()` function in AIEObjectFifoStatefulTransform
had a special case that returned 1 buffer when both the declared scalar
depth and the max acquire count were 1, suppressing the usual
`maxAcquire + 1` ping-pong increment. This exception was redundant
because the array-depth form (e.g. `[1, 1]`) already provides a way
to explicitly enforce per-endpoint depths. Remove the exception so that
a scalar depth-1 objectFifo consistently lowers to two buffers like any
other scalar depth, and document that `[1, 1]` is the correct way to
enforce a single buffer.

Changes:
- Remove the `maxAcquire == 1 && objFifo.size() == 1` guard from
  `findObjectFifoSize()` in AIEObjectFifoStatefulTransform.cpp
- Update `depth_one_objectfifo_test.mlir` to use `[1, 1]` array depth
  so the test continues to verify single-buffer lowering via the
  explicit array-depth mechanism
- Add `depth_one_scalar_test.mlir`: scalar depth 1 now lowers to 2
  buffers (ping-pong)
- Add `depth_one_array_test.mlir`: array depth `[1, 1]` still lowers
  to 1 buffer, confirming the enforcement mechanism works
- Add `depth_two_scalar_test.mlir`: scalar depth 2 with max acquire 1
  lowers to 2 buffers (not 3), confirming buffer count is driven by
  maxAcquire rather than the declared size
- Update `01_single_double_buffer/` example designs to use depth 2
  (ping-pong) and add a note that `[1, 1]` enforces a single buffer

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- depth_one_array/scalar and depth_two_scalar tests: shim tile consumers
  have no local memory, so no consumer buffers are created; update CHECKs
  to expect producer-side buffers only
- repeat_count tests: scalar depth-1 now applies the prefetch increment,
  so switch objectfifos to array depth [1, 1] to preserve single-buffer
  semantics required by the repeat_count feature on compute tiles

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 20, 2026

Coverage Report

Created: 2026-03-23 08:42

Click here for information about interpreting this report.

FilenameFunction CoverageLine CoverageRegion CoverageBranch Coverage
IR/AIEDialect.cpp 90.48% 86.04% 87.29% 78.89%
Transforms/AIEObjectFifoStatefulTransform.cpp 100.00% 94.79% 92.58% 86.16%
Totals 92.86% 90.22% 89.29% 81.87%
Generated by llvm-cov -- llvm version 18.1.3

abisca and others added 3 commits March 23, 2026 00:27
Three issues introduced after removing the depth-1 scalar exception:

1. Segfault in printObjectFifoInitValues when elemNumber is an ArrayAttr
   (e.g. [1, 1]) and initValues is set: dyn_cast to IntegerAttr returned
   null, crashing on .getInt(). Mirror the array-check already present
   in parseObjectFifoInitValues.

2. Memory overflow in bottleneck/resnet ML examples: weight objectFifos
   declared with scalar depth=1 now go through findObjectFifoSize() which
   returns maxAcquire+1=2, doubling weight buffer allocations and
   exceeding tile memory limits. Fix by using array-form depth [1, 1] so
   findObjectFifoSize() is bypassed.
   - IRON API: _get_depths() no longer collapses depth-1 to scalar,
     so ObjectFifo(depth=1) emits [1, 1] MLIR array form automatically.
   - Low-level API (_placed.py): explicitly pass [1, 1] / [1, 1, 1].
   - MLIR: change 1 : i32 to [1, 1] / [1, 1, 1] in resnet aie.mlir.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The rule is:
- Scalar depth in IRON emits scalar depth in MLIR, letting
  findObjectFifoSize() run during lowering (so depth=1 correctly
  becomes double-buffered, depth=2 stays 2, etc.)
- List depth in IRON emits array depth in MLIR, bypassing
  findObjectFifoSize() so the declared depths are used as-is.

Implement this by:
- Accepting list[int] for ObjectFifo(depth=...), storing an
  _array_depth flag and normalising _depth to a scalar for
  per-handle bounds checks.
- Restoring _get_depths() to collapse uniform scalar depths to a
  single int; only returning a list when _array_depth is set or
  depths differ across endpoints.
- Widening split(depths=...) to accept list[int | list[int]].

Update bottleneck.py and resnet.py weight fifos to depth=[1, 1]
so they stay single-buffered rather than being double-buffered by
findObjectFifoSize() after the depth-1 exception removal.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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