[Relay] Refactor Interpreter to treat lowering as IRModule->IRModule rewrite. - #8597
Merged
Merged
Conversation
mbs-octoml
requested review from
Huyuwei,
MarisaKirisame,
ZihengJiang,
anijain2305,
areusch,
comaniac,
jroesch,
junrushao,
jwfromm,
kazum,
liangfu,
masahi,
mbrookhart,
merrymercy,
siju-samuel,
slyubomirsky,
srkreddy1238,
tmoreau89,
tqchen,
vinx13,
wweic,
yzhliu and
zhiics
as code owners
July 30, 2021 00:11
Contributor
Author
|
@jroesch and @electriclilies for review |
mbs-octoml
marked this pull request as draft
July 30, 2021 00:14
mbs-octoml
force-pushed
the
mbs-tecompiler-refactoring
branch
from
July 30, 2021 18:34
9b894d7 to
feb036d
Compare
mbs-octoml
force-pushed
the
mbs-tecompiler-refactoring
branch
2 times, most recently
from
August 3, 2021 15:51
34e00fb to
01c73c5
Compare
mbs-octoml
force-pushed
the
mbs-tecompiler-refactoring
branch
3 times, most recently
from
August 5, 2021 23:28
bc233cd to
020af1a
Compare
altanh
reviewed
Aug 6, 2021
altanh
reviewed
Aug 6, 2021
altanh
reviewed
Aug 6, 2021
altanh
reviewed
Aug 6, 2021
altanh
reviewed
Aug 6, 2021
mbs-octoml
force-pushed
the
mbs-tecompiler-refactoring
branch
from
August 6, 2021 21:09
a602e4b to
1f741bd
Compare
Contributor
Author
|
@jroesch PTAL:
|
jroesch
reviewed
Aug 6, 2021
jroesch
reviewed
Aug 6, 2021
jroesch
reviewed
Aug 6, 2021
jroesch
reviewed
Aug 6, 2021
jroesch
approved these changes
Aug 6, 2021
jroesch
left a comment
Member
There was a problem hiding this comment.
LGTM, just some stuff we should do as follow up.
electriclilies
left a comment
Contributor
There was a problem hiding this comment.
Thanks for all your hard work Mark! The PR looks good to me so far, I really appreciate the high-quality comments and documentation. I'm still working my way through because it is quite long :)
electriclilies
approved these changes
Aug 6, 2021
electriclilies
left a comment
Contributor
There was a problem hiding this comment.
LGTM! Thanks again :)
mbs-octoml
force-pushed
the
mbs-tecompiler-refactoring
branch
from
August 8, 2021 15:25
1f741bd to
99951d8
Compare
mbs-octoml
force-pushed
the
mbs-tecompiler-refactoring
branch
3 times, most recently
from
August 11, 2021 22:01
8a83600 to
ccd62ef
Compare
mbs-octoml
force-pushed
the
mbs-tecompiler-refactoring
branch
2 times, most recently
from
August 16, 2021 22:14
d8fca6c to
f1fb28f
Compare
https://discuss.tvm.apache.org/t/rfc-relay-tecompiler-rewrite-existing-compile-engine-to-match-updated-compiler-flow/9233 This gets about halfway there for the Interpreter: * Remove direct access to TECompiler from interpreter, and instead call tec::LowerTEExpr when 'preparing' a module and expression for evaluation. * Make clear there's no phase distinction between create_interpreter and evaluate on the Python side -- both must be prepared together as a single IRModule. * But in return make sure the result of evaluate on the Python side is a packed func ready to directly apply 'simple' arguments to an already interpreted closure. * The interpreter builds and caches primitive TIR functions (and their corresponding dynamic shape functions) as packed funcs as they are encountered. * Cleanup uses of interpreter for constant folding on the C++ side. Future work: * Fold LoweredModule into IRModule so tec::LowerTEExpr is just another pass. * Get rid of the implicit caching of lowered functions in TECompiler. * Make calling convention from Relay to TIR explicit, and remove all the function attribute hackery currently needed so the interpreter can correctly invoke lowered functions as it encounters them. * Make TECompiler private. Though could do this now it will make migrating the VM and AOT uses of CompilerEngine harder. Force a gc between sphinx-gallery items to reclaim GPU memory. (apache#8722) GPU memory is only released once the PackedFunc for evaling the model is gced by Python. In CI we're noticing intermittent 'CUDA: Out of memory' failures while processing the tutorials, and tracing showed there was no gc happening between items. Not confident this will solve the problem but worth a try.
mbs-octoml
force-pushed
the
mbs-tecompiler-refactoring
branch
from
August 17, 2021 16:20
f1fb28f to
a249ea8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This continues the work outlined in
TECompiler: Rewrite existing compile engine to match updated compiler flow to centralize the TE->TIR lowering logic into a single IRModule->IRModule transformation. This gets about halfway there for the Interpreter:
Future work: