Simplify GPUTileSwizzleUtils and avoid creating unit dims. #19105
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
bjacob
requested review from
MaheshRavishankar,
qedawkins,
kuhar,
Groverkss,
antiagainst and
hanhanW
as code owners
bjacob
force-pushed
the
users/bjacob/cdna1-cdna2-data-tiling
branch
from
a89bc41 to
e5899d6
Compare
bjacob
force-pushed
the
users/bjacob/simplify-unroll
branch
from
7c3f370 to
d0eb097
Compare
hanhanW
reviewed
Nov 12, 2024
compiler/src/iree/compiler/Codegen/Dialect/GPU/IR/GPUTileSwizzleUtils.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Dialect/GPU/IR/GPUTileSwizzleUtils.cpp
Outdated
Show resolved
Hide resolved
Comment on lines
+79
to
+81
| // expected-remark @above {{layout of result #0 is #iree_vector_ext.layout<<[ BATCHX, LANEX], [1, 16]>, <[ BATCHY, VECTORX], [1, 16]>>}} | ||
| %rhs = vector.transfer_read %b[%c0, %c0], %cst_0 {in_bounds = [true, true]} : memref<16x16xf16>, vector<16x16xf16> | ||
| // expected-remark @above {{layout of result #0 is #iree_vector_ext.layout<<[ BATCHX, LANEX], [1, 16]>, <[ BATCHY, LANEY, VECTORX], [1, 1, 16]>>}} | ||
| // expected-remark @above {{layout of result #0 is #iree_vector_ext.layout<<[ BATCHX, LANEX], [1, 16]>, <[ BATCHY, VECTORX], [1, 16]>>}} |
There was a problem hiding this comment.
I did not know that the patch would update the test. I thought that the TileSwizzle things are only used on data-tiling path. Is it used by other work today?
There was a problem hiding this comment.
Yes, as of #18953, it is also used to compute iree_vector_ext.layout.
compiler/src/iree/compiler/Codegen/Dialect/GPU/IR/GPUTileSwizzleUtils.cpp
Outdated
Show resolved
Hide resolved
bjacob
force-pushed
the
users/bjacob/simplify-unroll
branch
from
d0eb097 to
47232f2
Compare
Base automatically changed from
users/bjacob/cdna1-cdna2-data-tiling
to
main
November 12, 2024 19:59
Signed-off-by: Benoit Jacob <[email protected]>
bjacob
force-pushed
the
users/bjacob/simplify-unroll
branch
from
47232f2 to
3081305
Compare
Groverkss
pushed a commit
to Groverkss/iree
that referenced
this pull request
Dec 1, 2024
…19105) In `getIntrinsicSwizzle`, we had a slightly roundabout way of constructing the swizzle from the `SingleSubgroupLayout`. We started from the `thread` dims, which we used unconditionally even if they had the value 1, leading to unit dims; and then we inserted the `element` dims *on the inside*, which required custom manipulation of the `swizzle` field. Now we just start from the `element` dims and work our way outwards from there, which means we can reuse the same helper that used to be named `unroll` and that we rename here to `expand` in preparation for iree-org#19102, and which we also move to be a `static` helper since it's no longer used outside of this file. --------- Signed-off-by: Benoit Jacob <[email protected]>
giacs-epic
pushed a commit
to giacs-epic/iree
that referenced
this pull request
Dec 4, 2024
…19105) In `getIntrinsicSwizzle`, we had a slightly roundabout way of constructing the swizzle from the `SingleSubgroupLayout`. We started from the `thread` dims, which we used unconditionally even if they had the value 1, leading to unit dims; and then we inserted the `element` dims *on the inside*, which required custom manipulation of the `swizzle` field. Now we just start from the `element` dims and work our way outwards from there, which means we can reuse the same helper that used to be named `unroll` and that we rename here to `expand` in preparation for iree-org#19102, and which we also move to be a `static` helper since it's no longer used outside of this file. --------- Signed-off-by: Benoit Jacob <[email protected]> Signed-off-by: Giacomo Serafini <[email protected]>
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.
In
getIntrinsicSwizzle, we had a slightly roundabout way of constructing the swizzle from theSingleSubgroupLayout. We started from thethreaddims, which we used unconditionally even if they had the value 1, leading to unit dims; and then we inserted theelementdims on the inside, which required custom manipulation of theswizzlefield. Now we just start from theelementdims and work our way outwards from there, which means we can reuse the same helper that used to be namedunrolland that we rename here toexpandin preparation for #19102, and which we also move to be astatichelper since it's no longer used outside of this file.