Add minimalistic implementation of subgraph matcher. #19322
Conversation
facebook-github-bot
added
the
oncall: jit
|
We can later add more features to this (e.g. nice iterators as in #17100). I'm putting this version for review now to unblock some of other work items. |
| } | ||
|
|
||
| /** | ||
| * Recursively try to match pattern with the actual graph starting from the |
There was a problem hiding this comment.
is this recursive?
Yes, matchNodes call matchValues, and matchValues call matchNodes. The recursion stops when we reach Param nodes in the pattern or if we hit an already visited node/value.
| while (!blocks_to_visit.empty()) { | ||
| const Block* block = blocks_to_visit.top(); | ||
| blocks_to_visit.pop(); | ||
| for (const Node* n : block->nodes()) { |
There was a problem hiding this comment.
maybe call out that this is a match on the imperative interpretation of the IR, and not the data flow graph
diamonds declared in different orders won't match, right?
There was a problem hiding this comment.
Hm, they should match whatever the order is (I added a test for this). We do follow data flow graph, not the sequential list of nodes: matchNodes call matchValues for nodes' inputs, and matchValues proceed to values' defs calling matchNodes for them.
PS: This implementation lacks aliasing checks, which are required if we're going to replace matched subgraphs with something else - I don't forget about it, just plan to follow up on that separately.
Add minimalistic implementation of subgraph matcher. gh-metadata: pytorch pytorch 19322 gh/ZolotukhinM/10/head
| } | ||
|
|
||
| /** | ||
| * Recursively try to match pattern with the actual graph starting from the |
There was a problem hiding this comment.
is this recursive?
Yes, matchNodes call matchValues, and matchValues call matchNodes. The recursion stops when we reach Param nodes in the pattern or if we hit an already visited node/value.
| while (!blocks_to_visit.empty()) { | ||
| const Block* block = blocks_to_visit.top(); | ||
| blocks_to_visit.pop(); | ||
| for (const Node* n : block->nodes()) { |
There was a problem hiding this comment.
Hm, they should match whatever the order is (I added a test for this). We do follow data flow graph, not the sequential list of nodes: matchNodes call matchValues for nodes' inputs, and matchValues proceed to values' defs calling matchNodes for them.
PS: This implementation lacks aliasing checks, which are required if we're going to replace matched subgraphs with something else - I don't forget about it, just plan to follow up on that separately.
Add minimalistic implementation of subgraph matcher. gh-metadata: pytorch pytorch 19322 gh/ZolotukhinM/10/head
Add minimalistic implementation of subgraph matcher. gh-metadata: pytorch pytorch 19322 gh/ZolotukhinM/10/head
Add minimalistic implementation of subgraph matcher. gh-metadata: pytorch pytorch 19322 gh/ZolotukhinM/10/head
|
@ZolotukhinM merged this pull request in 9818c7c. |
Summary: Pull Request resolved: pytorch#19322 ghimport-source-id: 93c713f829d1b2a9aa5d104cb1f30148dd37c967 Differential Revision: D14962182 Pulled By: ZolotukhinM fbshipit-source-id: 3989fba06502011bed9c24f12648d0baa2a4480c
Stack from ghstack:
Differential Revision: D14962182