- User Since
- Sep 16 2016, 11:47 AM (203 w, 4 d)
Thu, Aug 6
Wed, Aug 5
Thanks Frank! It looks good to me. Just a few minor comments
Tue, Aug 4
Mon, Aug 3
Thanks for the patch! Just a high-level comment for now. It looks good overall.
Tue, Jul 28
Mon, Jul 27
Thu, Jul 23
Wed, Jul 22
Tue, Jul 21
Mon, Jul 20
Jun 26 2020
Thanks Jeremy! Some minor comments.
Jun 25 2020
Jun 24 2020
Jun 23 2020
Thanks for fixing this! LGTM
Jun 15 2020
Jun 11 2020
Jun 10 2020
Jun 8 2020
Thanks Uday! Good point. I haven't looked too much into the slice computation but if you can send me some pointers and some examples on how the regions should be computed I could have a look. Thanks!
Jun 5 2020
If no more comments, I'll land this after https://reviews.llvm.org/D81302
Jun 4 2020
Jun 3 2020
Jun 2 2020
Jun 1 2020
May 29 2020
May 27 2020
Thanks @rriddle. I'll take care of this.
Thanks, LGTM. You can rename both intrinsics in a separate commit.
May 22 2020
Thanks! It looks good to me in general. I just added some comments about making it a bit more generic.
For my understanding, how do we plan to lower to these vector matrix multiply and transpose? From a vector contraction?
May 20 2020
It sounds good, thanks.
Replacing 'origin' with 'https://github.com/llvm/llvm-project.git' in git pull and push commands.
May 19 2020
Changing to git push origin HEAD:master
Adding Alex's changes. Thanks for the patch!
I'll proceed with the commit if no more comments.
Thank you so much for such a detailed explanation! It's really good! I think we should move this to somewhere in the documentation because it's really elucidating!
May 18 2020
I was thinking about doing the same! Thanks for doing this, Nicolas!
Thanks, Nicolas! It looks good! Just a minor comment below.
This patch introduces an attribute to set if a vector load/store is masked but how is that mask being computed/passed to the vector load/store?
IIUC, the current implementation assumes that there is no divergent control flow and therefore the loop exit condition can be used as mask for all the vector loads/stores, is that correct?
May 14 2020
I tried to change all the interface methods to have a default implementation but I'm hitting some problems. After reading again the documentation about interfaces, it's not clear to me what is the difference between providing a methodBody or a defaultImplementation. I had to dig into the generated code to have a bit better understanding but still, I couldn't make it work. I uploaded the patch with both approaches: methods in AffineWriteOpInterface has a methodBody, methods in AffineReadOpInterface has a defaultImplementation.
Provide a default implementation for interface methods (not working, see next message)
May 13 2020
Enabled multi-dim vector loads/stores (minor fixes)
Added tests and doc examples.
Thanks for the feedback!
This overall looks great. Please also update the commit title for vload/vstore -> vector_load/store. (since arc won't do it by itself as you know.)
- Added specific verifiers for affine.vector_load and affine.vector_store.
- Refactored affine.load and affine.store verifiers to avoid code duplication.
- Added invalid tests.
May 12 2020
Also: you did mention in the past that you were interested to look into gather/scatter load/store with some 2D native memory unit in mind, do you still plan to look into this later?
Addressing all the feedback. Thanks!
What about affine.vector_store (or affine.vec_store)?
May 11 2020
Addressing feedback. Thanks!
May 8 2020
A few things I found along the road...
May 7 2020
May 4 2020
Hi Kit. This should be it. Thanks for taking a look!
Apr 30 2020