[RELAY] Non-recursive Graph Vistor and Rewriter by mbrookhart · Pull Request #4886 · apache/tvm

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 · 2020-03-13T18:36:51Z

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

zhiics

LGTM:)

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

+    } 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?

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

Comment thread include/tvm/relay/expr_functor.h Outdated

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

Comment thread src/relay/ir/expr_functor.cc Outdated

Comment thread src/relay/analysis/util.cc Outdated

anijain2305 reviewed Mar 20, 2020

View reviewed changes

Comment thread src/relay/ir/expr_functor.cc Outdated

Comment thread src/relay/ir/expr_functor.cc Outdated

Comment thread src/relay/ir/expr_functor.cc Outdated

Comment thread src/relay/ir/expr_functor.cc Outdated

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

Comment thread include/tvm/relay/expr_functor.h Outdated

Comment thread src/relay/ir/expr_functor.cc

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

Conversation

mbrookhart commented Feb 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jroesch commented Mar 13, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tqchen commented Apr 3, 2020

Uh oh!

tqchen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zhiics left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

icemelon Apr 3, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tqchen commented Apr 3, 2020

Uh oh!

icemelon left a comment

Choose a reason for hiding this comment

Uh oh!

tqchen commented Apr 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

mbrookhart commented Feb 14, 2020 •

edited

Loading