User Details
- User Since
- Jan 6 2015, 1:11 PM (429 w, 3 d)
Tue, Mar 28
Mon, Mar 20
Rebase
Thu, Mar 16
Rebase.
Tue, Mar 14
Mon, Mar 13
Rebase again.
Rebase
Fri, Mar 3
Awesome! Thanks!
Mar 1 2023
Fix lit tests.
Fix bug.
Feb 17 2023
I am unblocking given it will be iterated on. Happy to chat offline later
Probably needs to be into NamedOpConversion pass
Feb 16 2023
Maybe we should file a bug to fix the issue with simplification
This seems strange to me. There is nothing that prevents a subsequent transformation to collapse the cast into the slice. Indeed it is probably better to do so.
Thanks!
Feb 15 2023
I know this pattern exists in IREE, but it is a bit of a hack. The representation of the im2col as a linalg.generic doesnt work as well. In reality it is similar to a gather. If this is upstreamed, it might be worth doing this right and not use an unnecessarily higher-dimensional operation for representing the im2col. Maybe we can chat offline?
Feb 12 2023
Feb 11 2023
Sure. Just push the test. Thanks!
Feb 9 2023
@Ding I tried to rebase this and land. Seems to fail tests.
I don't think so. I am AFK. I can merge when I get to my machine
I see why you made it a function pass, but I think it needs to be a pass on FunctionOpInterface. There are examples of this in IREE https://github.com/iree-org/iree/blob/55d0ffe7ea4ca5b58dd9197608151cc2f8c7521c/compiler/src/iree/compiler/Dialect/Flow/Transforms/Passes.td#L65 .
Have one comment here, more for my understanding of the nuances. Change itself looks good!
Jan 31 2023
Jan 30 2023
Address comments.
Jan 29 2023
Jan 26 2023
I think the core logic being fixed here is not something I worked on, so I dont have full context. Adding folks who worked on this as reviewers...
Jan 25 2023
This is pretty cool! It makes CSE very powerful. Last time I edited, I tried to walk a fine line and take small steps. So thanks for cleaning all of these up! This is taking a big step. I think its OK. But I'll probably wait for others to chime in.
Jan 24 2023
Could we just use the hoisting part here. You can just hoist operations outside of if and let standard CSE common out the users.... THis is duplicating a lot of functionality
Jan 23 2023
This is very painful... Maybe we can just hold off on this until it is needed....
Unblocking, but have one comment remaining.
Thanks!
Jan 20 2023
The pattern itself looks fine, but I think this has a dependency issue. There is already a Tensor -> Linalg dependence. This is adding a Linalg -> Tensor dependence.... That could be problematic... is there a way to do this as part of bufferization or something, or move these patterns to Linalg?
unblocking since there was a PSA. Thanks!
Looks good. THanks. Just a few nits.
Yeah, this is fine by me.
Jan 19 2023
Jan 17 2023
To start with, I see at minima design consistency with the general dialect design I think I saw upstream so far.
Thanks! This was the fix I had in mind.
Jan 16 2023
This patch had a use-after-free issue. Fixed with https://reviews.llvm.org/D141869
Jan 15 2023
Address comments.