Checking constant allocation sizes. Removing memref dialect usage.#43
Merged
Conversation
rainij
requested changes
May 8, 2026
Contributor
|
@RiccardoRomanello if the PR is related or even resolves an issue, please mention it in the description of the PR. |
Co-authored-by: Reinhard Stahn <51020828+rainij@users.noreply.github.com> Signed-off-by: Riccardo Romanello <riccardo.romanello@gmail.com>
EnriqueNB
requested changes
May 12, 2026
Collaborator
EnriqueNB
left a comment
EnriqueNB
left a comment
There was a problem hiding this comment.
I added a few comments about the formatting and simplification of the code. The logic look good to me. Please, consider them.
rainij
requested changes
May 12, 2026
rainij
reviewed
May 12, 2026
rainij
requested changes
May 18, 2026
Contributor
rainij
left a comment
rainij
left a comment
There was a problem hiding this comment.
@RiccardoRomanello you did not answer to some of my threads from the last review. Please have a look, you probably just missed them.
rainij
requested changes
May 20, 2026
Contributor
rainij
left a comment
rainij
left a comment
There was a problem hiding this comment.
Just a few tiny things left.
rainij
approved these changes
May 27, 2026
Contributor
|
@burgholzer this can be merged from my side. @EnriqueNB please have a look if your requests have been addressed. |
burgholzer
requested changes
May 27, 2026
Collaborator
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
Overall, this LGTM. Just two very small consistency comments and one slightly larger thought about leaving cleanup to canonicalization, which I believe is not necessarily a good idea for performance.
Applying statically applicable suggestions. Co-authored-by: Lukas Burgholzer <burgholzer@me.com> Signed-off-by: Riccardo Romanello <riccardo.romanello@gmail.com>
Contributor
|
The github UI tells me that there are still unresolved threads. I am absolutely not able to find those. No clue. Maybe it is about merging main, lets see if if this works. Update: didn't work. I merge anyway. |
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
4 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.
Added two passes into JaspToQC directory to: