[LowerToAIE] Refactor pass #705
Merged
Conversation
This file contains 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
newling
requested review from
MaheshRavishankar,
nirvedhmeshram,
yzhang93,
Abhishek-Varma and
jtuyls
as code owners
Abhishek-Varma
previously requested changes
Aug 27, 2024
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIELowerToAIE.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
jtuyls
requested changes
Aug 27, 2024
There was a problem hiding this comment.
iree-amdaie-sink-into-core
This pass will eventually be removed, once we can do the stateful transform on the amdaie dialect.
I don't think this is a limitation introduced by stateful trasnform, but needed to generate standalone code for the cores (through peano/chess), so this will still be needed?
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIELowerToAIE.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIESinkIntoCore.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/Passes.td
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/Passes.td
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
newling
force-pushed
the
refactor_lower_to_aie
branch
from
520660c to
d6c8dcc
Compare
Good point. I've moved it to act on amdaie.core. But in the process I realised using PatternRewriter for this is incredibly fragile unless the op we're sinking into can somehow be formally isolated from above. I've avoided PatternRewriter in the updated version, but we'll probably need to revisit this (in this PR or later). |
newling
force-pushed
the
refactor_lower_to_aie
branch
from
e2a1311 to
8c42d2a
Compare
jtuyls
reviewed
Aug 29, 2024
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIELowerToAIE.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/Passes.td
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/test/sink_into_core.mlir
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECanonicalizeNpuDmaCpyNd.cpp
Outdated
Show resolved
Hide resolved
newling
force-pushed
the
refactor_lower_to_aie
branch
from
8c42d2a to
1f2c628
Compare
|
Thanks for the review @jtuyls, this should really be 2 PRs sorry about that. |
newling
force-pushed
the
refactor_lower_to_aie
branch
from
1f2c628 to
2520721
Compare
newling
force-pushed
the
refactor_lower_to_aie
branch
from
2520721 to
9a7be93
Compare
newling
requested review from
Abhishek-Varma
and removed request for
Abhishek-Varma
newling
dismissed
Abhishek-Varma’s stale review
Dismissing, as I have addressed comments :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
It'll be good to lower directly from amdaie objectfifo operations, to aie lock/bd operations. This PR moves in that direction by moving logic out of aie to amdaie dialects. Specifically, the lower-to-aie pass gets split up, with 2 small passes being extracted (see passes.td for descriptions)
iree-amdaie-canonicalize-npu-dma-cpy-nd
This pass gets the zero stride dimension to the outermost dimension, and now acts on the amdaie dialect.
iree-amdaie-sink-into-core
This pass sinks dependencies into amdaie.core operations.