Relax SliceOp to being able to be a root op. #3876
Conversation
hanhanW
commented
Nov 17, 2020
•
hanhanW
commented
- Move mergable checks to DispatchConfig.
- Add a new trait for root only ops.
- Add slice + add tests.
| ++lhsResultIdx) { | ||
| if (rhsArgs[rhsOpIdx] == lhs.getResult(lhsResultIdx)) { | ||
| for (auto *user : rhsBlock.getArgument(rhsOpIdx).getUsers()) { | ||
| if (isa<mhlo::TorchIndexSelectOp>(user)) { |
There was a problem hiding this comment.
it'd be really nice to remove as many of these hardcoded ops as possible - is this still actually required to be inspected manually like this? can the trait you're looking for here be generalized (like isIndexableOp in dispatch analysis, etc) so that we have a chance of not ending up with an explosion of things in this file?
There was a problem hiding this comment.
if this kind of super specific check is needed, move it to DispatchConfig.h as an isFoo(op) method
There was a problem hiding this comment.
I plan to move it to DispatchConfig.h
For long-term, we would like to remove most of these limits, but now I'm trying to push the boundary so we will get more ideas where we are and how to push things forward.
There was a problem hiding this comment.
SGTM! what you're doing now is super useful as it lets us know which ops we actually care about and hopefully come up with some unique traits for them that we can upstream and apply to other frontend dialects like TOSA - way easier to do that when we have the minimal set and not a bunch of random ones :)
| for (int rhsOpIdx = 0; rhsOpIdx < rhsArgs.size(); ++rhsOpIdx) { | ||
| for (int lhsResultIdx = 0; lhsResultIdx < lhs.getNumResults(); | ||
| ++lhsResultIdx) { | ||
| if (rhsArgs[rhsOpIdx] == lhs.getResult(lhsResultIdx)) { |
There was a problem hiding this comment.
5 level deep nesting gets hard to read - prefer instead to early exit to keep things shallow
for example, if (rhs arg != lhs arg) continue;
hanhanW
force-pushed
the
leaf-root-op
branch
from
47da78f
to
3dcdd2f
Compare
|
Still need some clean up, please hold off the review a bit |
