[BACKEND] Cleanup redundant broadcast combine pattern #5167
Conversation
The triton-combine pass has a pattern that folds broadcast(cst) -> cst but this is redundant with the existing folder. Also, in the triton-reorder-broadcast pass we create a pattern which calls `Op::canonicalize`, but we can just use `Op::getCanonicalizationPatterns` instead.
peterbell10
force-pushed
the
pb/pattern-cleanup
branch
from
07dcfa7
to
3c84c26
Compare
| passes.common.add_canonicalizer(pm) | ||
| passes.ttir.add_combine(pm) |
There was a problem hiding this comment.
I'm swapping these passes around so that the combine pass can assume the input is canonicalized.
There was a problem hiding this comment.
I think that makes sense. I wonder if this will prevent some canonicalization to happen as broadcast ops may be in the middle. What kind of canonicalization do you need for this pass?
Worst case if we see problems we can run canonicalization one more time
There was a problem hiding this comment.
I think all the broadcast related things are now in canonicalize and combine is now mainly dot-related things, plus the addptr(addptr(ptr, a), b) -> addptr(ptr, a + b) pattern which shouldn't be effected by broadcasting really. So I think this is a good separation.
I do wonder if it might be worthwhile to add another canonicalize pass though, since LICM and loop unrollng could potentially connect patterns that were previously separated by region boundaries. The reorder broadcast pass could also connect arith patterns that were broken up by broadcasts and/or splats.
Just a hunch though, I don't have any examples in mind.
| if (matchPattern(val, m_Zero()) || matchPattern(val, m_AnyZeroFloat())) | ||
| return true; | ||
| // broadcast(constant_0) | ||
| if (auto bc = val.getDefiningOp<BroadcastOp>()) { |
There was a problem hiding this comment.
Since the input is now canonicalized, we don't need to match against non-canonical forms.
Summary of changes: - Remove `broadcast(cst) -> cst` from the triton-combine pass since it's redundant with the existing folder. - Reorder the triton-combine pass to come after the canonicalize pass, to simplify pattern matching - Cleanup patterns in triton-reorder-broadcast that called `Op::canonicalize` in favor of `Op::getCanonicalizationPatterns`.
Summary of changes:
broadcast(cst) -> cstfrom the triton-combine pass since it's redundant with the existing folder.Op::canonicalizein favor ofOp::getCanonicalizationPatterns.