[codegen][gpu] Adding conv filter layout fhwc to preprocessing pipeline #19974
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
jerryyin
requested review from
qedawkins and
MaheshRavishankar
as code owners
jerryyin
changed the title
|
Hmm looks like the regression tests for SDXL had a couple of runtime failures. That's a bit surprising to me. I'll rerun the jobs and see if it reproduces. |
I think there is numeric change |
|
Let me rebase, the base branch is slightly older now |
jerryyin
force-pushed
the
users/zyin/filter-fhwc-preprocessing
branch
from
0cc26c5 to
0be3371
Compare
|
I've got some nice suggestions about how to triage those model failures offline from @Max191 and @nirvedhmeshram, WIP on this. Summary of failures is indicating:
|
This was referenced Feb 24, 2025
jerryyin
added a commit
that referenced
this pull request
Feb 25, 2025
…d_to_intrinsics on convolution (#20073) The `pad_to_intrinsics` pass only support `linalg.conv2d` op with `nhwc_hwcf` layout of convolution. This has created inconvenience around taking advantage of other convolution variants for their performance potentials. Once such scenario is the IR from `conv_filter_to_channels_last` will populate `conv2d_nhwc_fhwc` represented by `linalg.generic`. This PR extend support of the `pad_to_intrinsics` pass such that other convolution variants including: - Those that are represented with `linalg.generic` - Other layouts such as (filter layout of) `fhwc` `fchw` This PR will unblock #19974, and allow us to continue to use `pad_to_intrinsics` as igemm padding kernel catch up in performance. --------- Signed-off-by: jerryyin <[email protected]>
jerryyin
force-pushed
the
users/zyin/filter-fhwc-preprocessing
branch
from
0be3371 to
bcc6d69
Compare
Signed-off-by: jerryyin <[email protected]>
|
Original mixed precision test is passing after rebase. Now |
jerryyin
force-pushed
the
users/zyin/filter-fhwc-preprocessing
branch
from
3ef18c3 to
b281af1
Compare
Signed-off-by: jerryyin <[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.
This PR is follow-up to #19739 and #19798. In previous PRs, we allowed convolution filter to be converted according to pipeline options and lower through igemm lowering pipeline. As @Max191's tuning on convolution demonstrated the performance benefit of this pass, we decide to plumb the pass by default in this follow-up PR.
In this PR, we turn this pass on by default sdxl inference pipeline
iree-preprocessing-transpose-convolution-pipeline. This will allow the pass to turn on in convolution related inferencing workloads. It will also pickfhwclayout by default if no option is explicitly set forfilter-layout.Credit to discussion from @Max191 and @nirvedhmeshram that we don't always want to turn this pass on because transpose on filter can still be an overhead in training workloads. This pass matches the location where
ConvertConvToChannelsLastis.