- User Since
- Sep 16 2016, 11:47 AM (340 w, 1 d)
Fri, Mar 24
it makes sense. I think sibling fusion fell behind when I generalized producer-consumer fusion as I didn't look too much into it at that time.
Thu, Mar 23
LGTM! Just a minor comment that you can fix before landing.
In the commit message would be even better :)
Could you please elaborate on what is going on? Why this is no longer necessary? What is the replacement?
Wed, Mar 22
LGTM. All my comments have been resolved. Please, wait for Mehdi to provide feedback on the remaining open questions/secondary PRs.
Mon, Mar 20
Thu, Mar 16
Wed, Mar 15
Tue, Mar 14
LGTM! A couple of comments.
Mon, Mar 13
Sun, Mar 12
Sat, Mar 11
I took a first look and this looks really promising! Thank you so much for working on this so quickly.
Before we move forward with deeper reviews and comments, do you plan to send and RFC? I think this interface may benefit from one as it will impact multiple dialects. It would recommend so even if it's just an informative one.
Fri, Mar 10
Thanks for addressing this! Good discussion!
Thu, Mar 9
Wed, Mar 8
Mon, Mar 6
LGTM, thanks! I'd love to see this also working for 2D vectors :)
Fri, Mar 3
Clang-format is not passing. Assuming the llvm configuration is used, I wonder if the problem is that your clang-format is outdated. Perhaps, the formatting issues that you see in https://reviews.llvm.org/D145160 are related only to your code and this diff wouldn't be necessary?
Sorry, I meant the diff context. If you look at the code below, it says "Context not available". You have to use git diff -U999999 if you are uploading a patch manually (take a look at the link I provided in my previous message). If you use Arcanist (see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line), it should take care of this automatically.
Thanks for the contributions! Could you please submit a patch with full context? https://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch
Build fails due to "mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp" not formatted (clang-format) properly in the current upstream branch. Not sure if the formatting change should go with this MR.
Thanks for the contribution! Just a couple of nits! LGTM
Feb 21 2023
I'm missing context here but do we want to restrict the ABIs to only those two? We are using ilp32, for example, and there are others (e.g., ilp32f, lp64, lp64f etc.).
Feb 20 2023
Feb 16 2023
Feb 14 2023
Awesome! I didn't know there was a fallback mechanism!
Feb 13 2023
Feb 8 2023
LGTM! Just a comment about the tests
Feb 7 2023
Feb 6 2023
Feb 3 2023
We are seeing some correctness issues when integrating this commit. We attempted to fix it here https://reviews.llvm.org/D143243 but it looks like something else is still not working.
For the following affine.apply:
Jan 30 2023
Jan 29 2023
Thanks a lot for the contribution!
Jan 26 2023
Thanks for the fix!
Jan 25 2023
Ouch... Thanks, Aart!
Jan 24 2023
Thanks for adding support for this! It looks great! A few comments
Jan 23 2023
Jan 20 2023
Thanks for the review!