Make ObjectFIFO fully dynamic -- remove custom unrolling logic, and add new unrolling pass#3298
Draft
andrej wants to merge 10 commits into
Draft
Make ObjectFIFO fully dynamic -- remove custom unrolling logic, and add new unrolling pass#3298andrej wants to merge 10 commits into
andrej wants to merge 10 commits into
Conversation
andrej
force-pushed
the
objfifo-dynamic
branch
2 times, most recently
from
1b72f97 to
e93a3a2
Compare
andrej
changed the title
andrej
force-pushed
the
objfifo-dynamic
branch
2 times, most recently
from
38001a1 to
3199b9c
Compare
Make aie.use_lock carry its lock count as an i32 SSA operand instead of an optional integer attribute, and emit runtime-computed lock counts in the dynamic (loop-preserving) objectFifo lowering via a per-(fifo,port) "held" counter. Passes that need a compile-time constant read it back through UseLockOp::getConstantValue(). The static loop-unrolling path (dynamic-objFifos=false) is left in place and still emits compile-time constant lock counts, so static and dynamic lock lowering coexist on this branch. Compiler code only; the corresponding test updates follow in the next commit.
Update lock-bearing goldens for the new i32 SSA aie.use_lock value form, add dynamic_runtime_lock_* tests covering the runtime "held"-counter lock bookkeeping, add a bad-input test for a non-constant use_lock value where a constant is required, and drop the dynamic_cyclostatic_* goldens that exercised the removed static peel/held-before-loop lock accounting.
Emit the per-(fifo,port) runtime held-element counters as single-element memref.alloca (tagged aie.held_counter) so acquire/release bookkeeping is plain load/store, then run mem2reg in-pass to promote them to SSA values. Hard-error if any counter survives promotion. Regenerate the dynamic objectFifo lock CHECK lines accordingly.
Co-authored-by: André Rösti <androsti@amd.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work in progress! Below description is aspirational:
Builds on #3308. Isolated Diff.
The PR removes the existing custom unrolling logic from the stateful transform pass. This means this pass now always emits dynamic code, with aforementioned runtime bookkeeping.
Doing the bookkeeping at runtime might have a small performance impact. To counter this, I add a new unrolling pass after the ObjectFIFO transform. This pass reuses existing MLIR infrastructure to unroll the loops containing the dynamic code. After unrolling, the compiler can statically reason about the generated bookkeeping code and, in the common, simple cases, fold them back into fully constant lock acquire/release values. This erases the runtime overhead for the common case.
This sort of "roundabout" way of first emitting dynamic code, then unrolling this code and folding constants back where possible has the advantage of simplifying the passes, and supporting all control flow (code that can't be reasoned about statically just remains dynamic and takes the potential performance hit of a couple extra instruction the core).
This should drastically simplify the code, as the ObjectFIFO lowering pass no longer performs any static analysis of loops and conditionals, and it allows arbitrary control flow. The static implementation had to reject cases it couldn't reason about.