-
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
[Relay][VM] Port VM, VM compiler, and Object into python #3391
Conversation
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.
overall lgtm.
assert expr, "either expr or self.mod should be not null." | ||
if isinstance(expr, Expr): | ||
self.mod[self.mod.entry_func] = expr | ||
assert expr is None |
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.
Is the assert still valid? If true we can remove this argument.
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.
Yes. But if we remove this argument, pylint will fail because it overwrites the API.
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.
Ah ok. it's the interface of Executor
, never mind.
src/relay/backend/vm/compiler.cc
Outdated
Target target = Target::Create("llvm"); | ||
if (const auto* f = runtime::Registry::Get("relay.backend.build")) { | ||
mod = (*f)(tvm::Array<LoweredFunc>(lowered_funcs.begin(), lowered_funcs.end()), target); | ||
class VMBuildModule : public runtime::ModuleNode { |
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.
Can we give this a different name? build_module
is a general pet-peeve of mine for a compiler it has no previous meaning and doesn't clearly illustrate what this does. Module is also an incredibly overloaded word in TVM and is pretty ambiguous at this point.
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.
For example VMCompiler is a much better name imo.
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
@icemelon9 should we revive the PR? |
We got a core dump on the unit test, PTAL. |
* tmp * Port vm and object to python * clean up * update vm build module * update * x * tweak * cleanup * update * fix rebase * Rename to VMCompiler * fix
* tmp * Port vm and object to python * clean up * update vm build module * update * x * tweak * cleanup * update * fix rebase * Rename to VMCompiler * fix
This PR aims to port VM functionality into python.
cc @jroesch @tqchen @wweic @zhiics @junrushao1994 @slyubomirsky