- User Since
- Jan 6 2015, 1:11 PM (337 w, 2 d)
Wed, Jun 16
Fix test error.
Rebase and address comments.
Tue, Jun 15
Rebase and address comments
Please use clang-format on the patch. There seems to be a lot of linter errors.
Mon, Jun 14
Fri, Jun 11
LGTM-ing this. But needs to rebased on ToT before landing.
Tue, Jun 8
So not related to this particular change, but does the fusion of operations where one of the operands was previously say a tensor<f32> and is now just f32 type work?
Thu, Jun 3
Wed, Jun 2
Ok, first initial skim of this patch. I actually didnt follow the core logic. Maybe we can chat offline about this.
Have a comment above (clicked enter early there). Will review more, but if we are going down this path, do we just replicate all the tests here for tensors with memrefs as well?
Interesting that this makes it work for memref types. A q
Tue, Jun 1
Had offline chat about resolving issues.
- Dropping the fusion at init_tensor will be done as a follow up.
- Use of pointer is the convention.
Overall looks ok to me. One comment in response to the question below.
Another minor nit: Why change signature from OpOperand & to OpOperand *. I prefer the former to the latter.
May 18 2021
May 17 2021
Nice catch! Looks good to me. Maybe wait for Justin to take a look as well.
Moving pattern to elementwise fusion pass.
May 15 2021
May 12 2021
May 11 2021
Sorry, found a good way to check for the case.
Thanks. Thats what I thought. It was for the case where unit-dimensions are all folded away. I wish there was a way to assert that in the code. (I think it is provable that this is always the case based on reshape semantics, but still having an assert that currIndices (in the code above) correspond to all unit-dimensions would be good.
Is it possible to get a small repro for the issue. Basically looking for the example where you hit this. I know why this might have happened, but want to make sure it is the expected case.
May 10 2021
Mostly looks fine. Couple of minor comments about use of SmallVector<T>.
Looking through this (will take me a bit), but just adding a "request change" till i finish my review.
May 7 2021
Actually, I take it back. I think this makes sense. Since the tile size is set to 0, the loop wont be generated. Then the non-parallel loops are filtered out.
I am not sure how this would interfact with the case where some loops are not tiled. Would be good to try out examples from here where some loops are not generated as scf.parallel when the tile size is set to 0. In the same way, if the tile size is set to 0, then then loop wont be tiled and therefore not distributed.
May 6 2021
Just one minor typo (unrelated to this change, but good to fix)
Address comments and rebase.
May 5 2021
(clicked accept by mistake)
THanks Thomas. Just one last comment.
It seems like some transposes are introduced to account for the variant of convolution seen in TOSA. Is the plan to eventually have that as a named op without having to do the transpose?
May 4 2021
May 3 2021
Address minor comments
Apr 28 2021
THe changes to DropUnitDims.cpp (and the test file) are pulled out from https://reviews.llvm.org/D101258. Landing those changes required changes to the subview -> load folding needed for SPIR-V path to make it handle the rank-reducing case.
Dropping the patterns that introduced rank-reduced versions of
subtensor and subtensor_insert. They need to be supported on the
Apr 27 2021
Seems like something that needs an interface. Anyway, this looks fine.
Apr 26 2021