[RELAY] Non-recursive Graph Vistor and Rewriter #4886

mbrookhart · 2020-02-14T18:52:02Z

This PR is Work in Progress and under RFC

https://discuss.tvm.ai/t/performing-relay-passes-non-recursively/5696

@jroesch @tqchen

include/tvm/relay/expr_functor.h

jroesch · 2020-03-13T18:36:51Z

cc @MarisaKirisame

src/relay/ir/expr_functor.cc

src/relay/analysis/util.cc

src/relay/ir/expr_functor.cc

autoformat remove a currently empty test until testing is solidfied

passes tests when disabling GetExprRefCount, I think I have a bug in visit counting fix GetExprRefCount Fix a subtle bug with nested recursive/non-recursive scopes

fix a clang warning fix cpp lint fix doc param error

tqchen · 2020-04-03T15:42:24Z

@zhiics @jroesch @anijain2305 @yzhliu @icemelon9 please take a final look and https://tvm.apache.org/docs/contribute/code_review.html#approve-and-request-changes-explicitly

tqchen

some final comments, lgtm from my side once they are fixed

include/tvm/relay/expr_functor.h

src/relay/ir/expr_functor.cc

zhiics

LGTM:)

include/tvm/relay/expr_functor.h

icemelon · 2020-04-03T16:57:46Z

src/relay/ir/expr_functor.cc

+    } else if (const TupleGetItemNode* op = node.as<TupleGetItemNode>()) {
+      stack.top().second = true;
+      fpush_to_stack(op->tuple);
+    } else {


Should we expand LetNode as it could easily lead to deep dataflow?

include/tvm/relay/expr_functor.h

tqchen · 2020-04-03T17:20:51Z

cc @icemelon9 re let node, let us not expand letnode for now, as they could be useful in Dataflow normal form and marks boundries of the dataflow regions and effect

…ator-tvm into mbrookhart/GraphVisitor

icemelon

lgtm

tqchen · 2020-04-03T21:36:45Z

Thanks @icemelon9 @yzhliu @anijain2305 @jroesch @MarisaKirisame, and @mbrookhart ! It would be great if we can followup to convert more passes to the non-recursive form

* First pass a defining a non-recursive Graph Vistor and Rewriter autoformat remove a currently empty test until testing is solidfied * Make CalcDep from Dead Code Elimination non-recursive * Partially working, not passing all tests yet passes tests when disabling GetExprRefCount, I think I have a bug in visit counting fix GetExprRefCount Fix a subtle bug with nested recursive/non-recursive scopes * Refactor * improve comments * respond to review comments on comments * Fix a problem with default recursion for dataflow nodes mark DataflowVisitor methods as override * implement ScopeMutator * convert forward_rewrite to ScopeMutator, remove DataflowMutator * rewrite ExprRewriter and convert fast_math to use it * switch BiasAddSimplifier to ExprRewriter fix a clang warning fix cpp lint fix doc param error * respond to review comments * fix a typo in the iterative looping * add a regression test for GetExprRefCount issue * Normalize naming * fix lint * First pass a defining a non-recursive Graph Vistor and Rewriter autoformat remove a currently empty test until testing is solidfied * Make CalcDep from Dead Code Elimination non-recursive * Partially working, not passing all tests yet passes tests when disabling GetExprRefCount, I think I have a bug in visit counting fix GetExprRefCount Fix a subtle bug with nested recursive/non-recursive scopes * Refactor * improve comments * respond to review comments on comments * Fix a problem with default recursion for dataflow nodes mark DataflowVisitor methods as override * implement ScopeMutator * convert forward_rewrite to ScopeMutator, remove DataflowMutator * rewrite ExprRewriter and convert fast_math to use it * switch BiasAddSimplifier to ExprRewriter fix a clang warning fix cpp lint fix doc param error * respond to review comments * fix a typo in the iterative looping * add a regression test for GetExprRefCount issue * Normalize naming * fix lint * respond to review comments

mbrookhart changed the title ~~First pass a defining a non-recursive Graph Vistor and Rewriter~~ First pass a defining at non-recursive Graph Vistor and Rewriter Feb 14, 2020

mbrookhart force-pushed the mbrookhart/GraphVisitor branch from ab7eb8b to 450b998 Compare February 14, 2020 19:00

tqchen changed the title ~~First pass a defining at non-recursive Graph Vistor and Rewriter~~ [WIP][POC]First pass a defining at non-recursive Graph Vistor and Rewriter Feb 14, 2020

tqchen reviewed Feb 14, 2020

View reviewed changes

include/tvm/relay/expr_functor.h Outdated Show resolved Hide resolved

tqchen assigned jroesch and tqchen Feb 14, 2020

mbrookhart force-pushed the mbrookhart/GraphVisitor branch from 6933b30 to 9c952ee Compare February 28, 2020 22:39

mbrookhart force-pushed the mbrookhart/GraphVisitor branch 3 times, most recently from a13af48 to 4cf2eca Compare March 13, 2020 16:58

mbrookhart force-pushed the mbrookhart/GraphVisitor branch 4 times, most recently from 8785019 to 8871e0b Compare March 18, 2020 21:27

yzhliu reviewed Mar 19, 2020

View reviewed changes

src/relay/ir/expr_functor.cc Outdated Show resolved Hide resolved

src/relay/analysis/util.cc Outdated Show resolved Hide resolved

anijain2305 reviewed Mar 20, 2020

View reviewed changes

src/relay/ir/expr_functor.cc Outdated Show resolved Hide resolved

src/relay/ir/expr_functor.cc Outdated Show resolved Hide resolved

src/relay/ir/expr_functor.cc Outdated Show resolved Hide resolved

src/relay/ir/expr_functor.cc Outdated Show resolved Hide resolved

mbrookhart force-pushed the mbrookhart/GraphVisitor branch 2 times, most recently from 57c6531 to 62cddf2 Compare March 23, 2020 19:41

mbrookhart force-pushed the mbrookhart/GraphVisitor branch from 62cddf2 to 55a4213 Compare March 30, 2020 20:11

tqchen added status: need review status: need update need update based on feedbacks labels Mar 30, 2020

mbrookhart force-pushed the mbrookhart/GraphVisitor branch 2 times, most recently from cec3bae to 9c6a04d Compare March 31, 2020 15:52

Matthew Brookhart added 6 commits March 31, 2020 13:55

First pass a defining a non-recursive Graph Vistor and Rewriter

bd14b04

autoformat remove a currently empty test until testing is solidfied

Make CalcDep from Dead Code Elimination non-recursive

bfabd87

Partially working, not passing all tests yet

fd03421

passes tests when disabling GetExprRefCount, I think I have a bug in visit counting fix GetExprRefCount Fix a subtle bug with nested recursive/non-recursive scopes

Refactor

31d0664

improve comments

7a68f0e

respond to review comments on comments

6e4f7eb

mbrookhart and others added 8 commits April 2, 2020 16:11

convert forward_rewrite to ScopeMutator, remove DataflowMutator

4934eae

rewrite ExprRewriter and convert fast_math to use it

e08119c

switch BiasAddSimplifier to ExprRewriter

b1a496b

fix a clang warning fix cpp lint fix doc param error

respond to review comments

c1fc3e8

fix a typo in the iterative looping

ada8bd1

add a regression test for GetExprRefCount issue

509a087

Normalize naming

a7c8429

fix lint

ae3c62e

mbrookhart force-pushed the mbrookhart/GraphVisitor branch from 62ca9f5 to ae3c62e Compare April 2, 2020 23:11

mbrookhart changed the title ~~[WIP][POC]First pass a defining at non-recursive Graph Vistor and Rewriter~~ [POC]First pass a defining at non-recursive Graph Vistor and Rewriter Apr 3, 2020

tqchen approved these changes Apr 3, 2020

View reviewed changes

include/tvm/relay/expr_functor.h Outdated Show resolved Hide resolved

src/relay/ir/expr_functor.cc Show resolved Hide resolved

tqchen changed the title ~~[POC]First pass a defining at non-recursive Graph Vistor and Rewriter~~ [RELAY] Non-recursive Graph Vistor and Rewriter Apr 3, 2020

zhiics approved these changes Apr 3, 2020

View reviewed changes

icemelon reviewed Apr 3, 2020

View reviewed changes

mbrookhart added 2 commits April 3, 2020 10:40

Merge branch 'mbrookhart/GraphVisitor' of github.com:mbrookhart/incub…

dc89877

…ator-tvm into mbrookhart/GraphVisitor

respond to review comments

517e83d

icemelon approved these changes Apr 3, 2020

View reviewed changes

tqchen merged commit 7de8a53 into apache:master Apr 3, 2020

tqchen added status: accepted and removed status: need review status: need update need update based on feedbacks labels Apr 3, 2020

mbrookhart deleted the mbrookhart/GraphVisitor branch April 3, 2020 21:48

tqchen mentioned this pull request Jul 21, 2020

[Relay] Dead code elimination pass blows up call stack #4534

Closed

ZihengJiang mentioned this pull request Sep 25, 2020

TVM v0.7 Release Note Candidate #6486

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RELAY] Non-recursive Graph Vistor and Rewriter #4886

[RELAY] Non-recursive Graph Vistor and Rewriter #4886

mbrookhart commented Feb 14, 2020 •

edited

jroesch commented Mar 13, 2020

tqchen commented Apr 3, 2020

tqchen left a comment

zhiics left a comment

icemelon Apr 3, 2020

tqchen commented Apr 3, 2020

icemelon left a comment

tqchen commented Apr 3, 2020

[RELAY] Non-recursive Graph Vistor and Rewriter #4886

[RELAY] Non-recursive Graph Vistor and Rewriter #4886

Conversation

mbrookhart commented Feb 14, 2020 • edited

jroesch commented Mar 13, 2020

tqchen commented Apr 3, 2020

tqchen left a comment

Choose a reason for hiding this comment

zhiics left a comment

Choose a reason for hiding this comment

icemelon Apr 3, 2020

Choose a reason for hiding this comment

tqchen commented Apr 3, 2020

icemelon left a comment

Choose a reason for hiding this comment

tqchen commented Apr 3, 2020

mbrookhart commented Feb 14, 2020 •

edited