-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VM] Move param bind to OptimizeModule #7451
Conversation
This makes a lot more sense to me, thanks Masa. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Could you add a test case for this?
be0b48e
to
81f61b7
Compare
@icemelon9 Added a test to verify that all params are bound in the output of |
@icemelon9 ready to merge |
2847fbd
to
db2530a
Compare
db2530a
to
15385a4
Compare
* [VM] Move param bind to OptimizeModule * add test to verify the number of free vars after opt * remove const from OptimizeModule
* [VM] Move param bind to OptimizeModule * add test to verify the number of free vars after opt * remove const from OptimizeModule
In VM,
BindParamsByName
is run insideVMCompiler::Lower(...)
beforeOptimizeModule
gets called. So when we try to dump the optimized module like this:param binding doesn't happen, so the "optimized graph" has parameters as arguments. This means constant folding involving parameters cannot happen, so the optimized graph looks very different (e.g. there are many
layout_transform
) from the output ofrelay.optimize
, which does param binding whenrelay.optimize
is called.tvm/src/relay/backend/build_module.cc
Lines 255 to 263 in 384714b
This PR moves param binding in VM to
OptimizeModule
, to be consistent with the graph codegen. NowVMCompiler.optimize(...)
returns the graph with constants properly folded.please review @zhiics @jroesch @icemelon9