You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Work uncovered in #3553 that did not fit the scope, and additional "nice-to-have"s, copied from the PR there to here:
API Clarity
Untangle repeat_count and iter_count. The two are unrelated but entangled. They currently roughly mean the following: repeat_count is a temporal repetition of each segment, i.e. duplicate each segment N times before moving on. For multi-BD chains, this is achieved by creating N copies of the BD. iter_count is unrelated: it describes how long the transfer should run for. This attribute allows creating a bounded BD chain, and indicates the number of times the whole chain should execute. Deriving one from the other is confusing. Proposed semantics: rename repeat_count to replicate to avoid confusion with the unrelated hardware "repeat count" register. replicate = N means each object is transferred N times; Refactor iter_count = M means the chain runs M times and then stops; neither is ever reinterpreted as the other. (FIXME in place.)
Replace the "constant-data fifo" lock heuristic with the real question. Allocation currently skips locks when a pool starts full and has iter_count > 1. What actually decides it is "no endpoint fills this pool", which is now a question the IR can answer directly. (FIXME in place.)
Clarify ObjectFifo depth semantics. There is some max_acquire logic on the cores that adjusts how many buffers are allocated for a fifo based on the highest-seen acquire count. Previously, this could make an object fifo 'deeper' than was declared; this is now rejected. However, it can still make a fifo 'shallower' than requested in the 'depth' attribute, and it is somewhat inconsistent. Worth looking into, but probably fine as-is. See also test broadcast_self_adjusted_depths.mlir which this PR modified.
Clean up packet-flow command-line argument. Currently, this forces everything to be a packet flow with no per-flow option to opt-out to a circuit flow. When unset, it allows individual flows to opt-in to packet flow. A cleaner behavior would be to make the command-line argument set the per-attribute default.
Investigate ObjectFifo options to see if you can express the same things with a more concise set of options; this should also simplify checks for conflicts between options.
Separation of concerns
Move buffer placement out of --aie-objectfifo-allocate. Choosing when/where to spill a buffer is an allocator concern from which other passes could benefit too, not a ObjectFifo concern. This would require specifying affinity of a buffer (which tiles need access to it), and the allocator would then be free to spill it while respecting all tiles that need access. (Pre-existing behaviour, carried over unchanged; FIXME in place.)
Factor packet-ID assignment into its own pass. Assigning packet IDs is not a concern of the objectFifo allocator. A Packet ID allocation pass may create a new op %id = aie.packet_id value that aie.packet_flow takes as an argument, concretized by a pass of its own. (Pre-existing behavior carried over in this PR, FIXME in place.)
Follow-on work / extensions
Python bindings and examples for mid-level representation. Add examples, and if needed nice-ify Python bindings, for the new mid-level ops. See if any of the existing examples can be simplified using them.
Investigate if direct core access from a core to distribute/join buffers makes sense. The PR enabled distribute/join across core tiles -- using the DMAs of the core, but no direct access to the buffers on the core itself. Would it make sense to extend this so that if a distribute/join ends on a core tile, that core tile can directly access its buffers (using slices into memrefs)?
Other/infrastructure
Extend the clang-format pre-commit hook to .td files, and pin it to CI's version. The hook is scoped types_or: [c, c++], so tablegen is never checked locally, and it pins a different clang-format major than the git clang-format origin/main check in CI (22.x vs 20.1.0).
Work uncovered in #3553 that did not fit the scope, and additional "nice-to-have"s, copied from the PR there to here:
API Clarity
Untangle
repeat_countanditer_count. The two are unrelated but entangled. They currently roughly mean the following:repeat_countis a temporal repetition of each segment, i.e. duplicate each segmentNtimes before moving on. For multi-BD chains, this is achieved by creatingNcopies of the BD.iter_countis unrelated: it describes how long the transfer should run for. This attribute allows creating a bounded BD chain, and indicates the number of times the whole chain should execute. Deriving one from the other is confusing. Proposed semantics: renamerepeat_counttoreplicateto avoid confusion with the unrelated hardware "repeat count" register.replicate = Nmeans each object is transferred N times; Refactoriter_count = Mmeans the chain runs M times and then stops; neither is ever reinterpreted as the other. (FIXME in place.)Replace the "constant-data fifo" lock heuristic with the real question. Allocation currently skips locks when a pool starts full and has
iter_count > 1. What actually decides it is "no endpoint fills this pool", which is now a question the IR can answer directly. (FIXME in place.)Clarify ObjectFifo depth semantics. There is some
max_acquirelogic on the cores that adjusts how many buffers are allocated for a fifo based on the highest-seen acquire count. Previously, this could make an object fifo 'deeper' than was declared; this is now rejected. However, it can still make a fifo 'shallower' than requested in the 'depth' attribute, and it is somewhat inconsistent. Worth looking into, but probably fine as-is. See also testbroadcast_self_adjusted_depths.mlirwhich this PR modified.Clean up packet-flow command-line argument. Currently, this forces everything to be a packet flow with no per-flow option to opt-out to a circuit flow. When unset, it allows individual flows to opt-in to packet flow. A cleaner behavior would be to make the command-line argument set the per-attribute default.
Choose
dimensionsorsizes+stridesfor user-facing operations #1922Investigate ObjectFifo options to see if you can express the same things with a more concise set of options; this should also simplify checks for conflicts between options.
Separation of concerns
Move buffer placement out of
--aie-objectfifo-allocate. Choosing when/where to spill a buffer is an allocator concern from which other passes could benefit too, not a ObjectFifo concern. This would require specifying affinity of a buffer (which tiles need access to it), and the allocator would then be free to spill it while respecting all tiles that need access. (Pre-existing behaviour, carried over unchanged; FIXME in place.)Factor packet-ID assignment into its own pass. Assigning packet IDs is not a concern of the objectFifo allocator. A Packet ID allocation pass may create a new op
%id = aie.packet_idvalue thataie.packet_flowtakes as an argument, concretized by a pass of its own. (Pre-existing behavior carried over in this PR, FIXME in place.)Follow-on work / extensions
Python bindings and examples for mid-level representation. Add examples, and if needed nice-ify Python bindings, for the new mid-level ops. See if any of the existing examples can be simplified using them.
Investigate if direct core access from a core to distribute/join buffers makes sense. The PR enabled distribute/join across core tiles -- using the DMAs of the core, but no direct access to the buffers on the core itself. Would it make sense to extend this so that if a distribute/join ends on a core tile, that core tile can directly access its buffers (using slices into memrefs)?
Other/infrastructure
clang-formatpre-commit hook to.tdfiles, and pin it to CI's version. The hook is scopedtypes_or: [c, c++], so tablegen is never checked locally, and it pins a different clang-format major than thegit clang-format origin/maincheck in CI (22.x vs 20.1.0).