[GPU] Move tile and distribute pass before packing to intrinsic for TileAndfuse pipeline #19053
Conversation
nirvedhmeshram
requested review from
MaheshRavishankar,
qedawkins,
kuhar,
Groverkss,
antiagainst and
hanhanW
as code owners
compiler/src/iree/compiler/Codegen/Common/PropagateReshapesByExpansion.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/PropagateReshapesByExpansion.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/PropagateReshapesByExpansion.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/PropagateReshapesByExpansion.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/PropagateReshapesByExpansion.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/PropagateReshapesByExpansion.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/PropagateReshapesByExpansion.cpp
Outdated
Show resolved
Hide resolved
|
This seems like a hack... Can you give an example where it is required to TileAndDistribute first before packing to intrinsics to make it work for unaligned shapes? |
I think how it is right now, is actually the hack done by this PR #18565 The basic problem I was facing after that PR is for unaligned shapes tile and distribute introduces the extract slice but if we do the operand promotion which happens before pack to intrinsic then that logic doesnt quite work. @qedawkins can you please elaborate on why we broadly want the sequence that this PR is doing? |
I think that exactly what we should be doing. After you distribute you promote the operands and read into a padded shared memory buffer |
There was a problem hiding this comment.
Overall, looks good. I thought we may have to rethink the pack/unpack decomposition passes, but if tests are passing then it is probably fine.
Main comment is that the IGEMM pass should still run before workgroup distribution. I'm also curious why one of the TileAndFuse pipeline tests has changed.
compiler/src/iree/compiler/Codegen/LLVMGPU/test/ROCDL/pipeline_tile_and_fuse.mlir
Outdated
Show resolved
Hide resolved
…ileAndfuse pipeline Signed-off-by: Nirvedh <[email protected]> Signed-off-by: Nirvedh Meshram <[email protected]>
nirvedhmeshram
force-pushed
the
tile_and_fuse_do_workgroup_distribution_first
branch
from
6c83f21 to
0933cc1
Compare
|
@Max191 all tests are passing cleanly now due to improving the hoisting pattern and correcting the distribution to happen after IGEMM conversion :) |
…TileAndfuse pipeline (iree-org#19053) We want to first distribute to workgroups so that we can promote operands to handle unaligned to intrinsic cases before we concretize the mma shapes. Signed-off-by: Nirvedh Meshram <[email protected]>
…TileAndfuse pipeline (iree-org#19053) We want to first distribute to workgroups so that we can promote operands to handle unaligned to intrinsic cases before we concretize the mma shapes. Signed-off-by: Nirvedh Meshram <[email protected]> Signed-off-by: Giacomo Serafini <[email protected]>
We want to first distribute to workgroups so that we can promote operands to handle unaligned to intrinsic cases before we concretize the mma shapes.