Review annotations and test for allocations in generic matmatmul #52298
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
|
@jishnub You may wish to review the annotations in the matvecmul case accordingly. |
dkarrasch
commented
Nov 25, 2023
| @@ -567,7 +566,7 @@ function gemm_wrapper(tA::AbstractChar, tB::AbstractChar, | |||
| end | |||
| end | |||
|
|
|||
| Base.@constprop :aggressive function gemm_wrapper!(C::StridedVecOrMat{T}, tA::AbstractChar, tB::AbstractChar, | |||
| function gemm_wrapper!(C::StridedVecOrMat{T}, tA::AbstractChar, tB::AbstractChar, | |||
There was a problem hiding this comment.
The other function calls are behind dynamic branches, so it doesn't seem like constants are propagated through to matmul2x2!, matmul3x3! and _generic_matmatmul!, unless I'm mistaken. At least, it seems that calling gemm_wrapper! with 2x2 arguments, the whole matmul2x2! gets compiled, not only one branch.
dkarrasch
changed the title
|
@nanosoldier |
|
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. |
fredrikekre
reviewed
Dec 6, 2023
dkarrasch
added a commit
that referenced
this pull request
Dec 6, 2023
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 adds a few comments as to why compiler annotations are helpful, removes seemingly unhelpful annotations and adds tests to make sure we don't allocate. This is open to further suggestions for tests and/or the return of removed annotations for good reasons.