Support misaligned matmul using vectorization analysis#2105
Merged
jacobhinkle merged 52 commits intomainfrom May 13, 2024
Merged
Support misaligned matmul using vectorization analysis#2105jacobhinkle merged 52 commits intomainfrom
jacobhinkle merged 52 commits intomainfrom
Conversation
Needs tests. Also currently not using epilogue vec_size
This is also propagated backward to C inputs
Added utility to help plugins determine whether that setting is safe.
jacobhinkle
commented
Apr 18, 2024
Some tests still fail execution because there is a possible bug in operand vectorization.
jacobhinkle
commented
Apr 18, 2024
This fixes the analysis by analyzing per _row_ of operand.
jacobhinkle
changed the title
jacobhinkle
commented
Apr 19, 2024
This fixes an error where we were using incompatible kernels.
jacobhinkle
commented
Apr 19, 2024
jacobhinkle
commented
Apr 19, 2024
Collaborator
Author
|
!build --diff |
zasdfgbnm
reviewed
Apr 19, 2024
This changes SchedulerRuntimeInfo in a way that I believe is safe, but it will also require some review.
zasdfgbnm
reviewed
May 7, 2024
Collaborator
Author
|
!build |
zasdfgbnm
approved these changes
May 9, 2024
Collaborator
There was a problem hiding this comment.
I still think maxRowVectorization needs change, but consider that:
- The rest of this PR LGTM
- Even as-is, this PR is a strict improvement on what we have on main
- I will be off tomorrow and next week, I don't want to block this for one week.
I am OK with merging this as-is and fix maxRowVectorization in a future PR.
Collaborator
Author
|
!build |
zasdfgbnm
reviewed
May 9, 2024
zasdfgbnm
reviewed
May 9, 2024
Co-authored-by: Gao, Xiang <qasdfgtyuiop@gmail.com>
zasdfgbnm
reviewed
May 9, 2024
Collaborator
Author
|
!build |
This got lost in the shuffle a couple commits ago
Collaborator
Author
|
!build |
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
2 participants
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 change collects the maximum supported vectorization size for A, B, and "epilogue" and attaches these to
MatmulParams. In case any "C" role tensors like bias are provided, they are considered when computing epilogue vectorization. This information becomes available to heuristic plugins also.Fixes #2083. Fixes #983. Related to existing PR #807. (Related to issue #682 but I'm not sure yet if this approach will fix it).