[TensorExpr] Fuser: try merging adjacent fusion groups. #43671
Conversation
[ghstack-poisoned]
facebook-github-bot
added
the
oncall: jit
Original PR: #43671 [ghstack-poisoned]
…sion groups." Original PR: #43671 [ghstack-poisoned]
…sion groups." Original PR: #43671 [ghstack-poisoned]
…sion groups." Original PR: #43671 [ghstack-poisoned]
…sion groups." Original PR: #43671 [ghstack-poisoned]
Differential Revision: [D23360796](https://our.internmc.facebook.com/intern/diff/D23360796) [ghstack-poisoned]
Differential Revision: [D23360796](https://our.internmc.facebook.com/intern/diff/D23360796) [ghstack-poisoned]
💊 CI failures summary and remediationsAs of commit ad1ad5e (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 22 times. |
…sion groups." Original PR: #43671 [ghstack-poisoned]
…sion groups." Original PR: #43671 [ghstack-poisoned]
| } | ||
| } | ||
|
|
||
| void testFuserPass_4() { |
There was a problem hiding this comment.
nit: maybe testFuserPass_merge_adjacent_fusion_groups
| // If merging suceeded, save the merged group as the "previous" fusion | ||
| // group so that we can try to merge the next one into it. | ||
| if (prev_fusion_group) { | ||
| debugDumpFusionGroup( |
There was a problem hiding this comment.
small nit: theres a debug dump on each branch of the following if statement and a debug on the previous line, not sure how much information this adds
| debugDumpFusionGroup( | ||
| "Trying to merge into the previous fusion group: ", | ||
| prev_fusion_group); | ||
| if (canMerge(prev_fusion_group, fusion_group)) { |
There was a problem hiding this comment.
tryMerge can fail even when canMerge can fail, which is a little confusing. https://github.com/pytorch/pytorch/blob/master/torch/csrc/jit/passes/tensorexpr_fuser.cpp#L443
Can we inline canMerge into tryMerge, and then call tryMerge here instead of canMerge ? (these are the only two callsites)
|
|
||
| // Symbolic checks | ||
| REQ(canHandle(producer)); | ||
| REQ(canHandle(producer) || producer->kind() == prim::TensorExprGroup); |
There was a problem hiding this comment.
could also add prim::TensorExprGroup to the symbols which we support
Differential Revision: [D23360796](https://our.internmc.facebook.com/intern/diff/D23360796) [ghstack-poisoned]
Codecov Report
@@ Coverage Diff @@
## gh/ZolotukhinM/301/base #43671 +/- ##
==========================================================
Coverage ? 68.07%
==========================================================
Files ? 384
Lines ? 49765
Branches ? 0
==========================================================
Hits ? 33880
Misses ? 15885
Partials ? 0 Continue to review full report at Codecov.
|
Differential Revision: [D23360796](https://our.internmc.facebook.com/intern/diff/D23360796) [ghstack-poisoned]
Differential Revision: [D23360796](https://our.internmc.facebook.com/intern/diff/D23360796) [ghstack-poisoned]
Differential Revision: [D23360796](https://our.internmc.facebook.com/intern/diff/D23360796) [ghstack-poisoned]
Differential Revision: [D23360796](https://our.internmc.facebook.com/intern/diff/D23360796) [ghstack-poisoned]
|
@ZolotukhinM merged this pull request in d66520b. |
Summary: Pull Request resolved: #43671 Test Plan: Imported from OSS Reviewed By: eellison Differential Revision: D23360796 Pulled By: ZolotukhinM fbshipit-source-id: 60ec318fe77ae9f2c821d9c4d106281845266e0f
Original PR: pytorch/pytorch#43671 ghstack-source-id: cb085e1 Pull Request resolved: pytorch/pytorch#43802
ghstack-source-id: ff65088 Pull Request resolved: pytorch/pytorch#43671
Stack from ghstack:
Differential Revision: D23360796