You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/07/19 19:24:41 UTC
[GitHub] [tvm] mikepapadim opened a new pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
mikepapadim opened a new pull request #8501:
URL: https://github.com/apache/tvm/pull/8501
This is WIP following the RFC in https://discuss.tvm.apache.org/t/rfc-relay-tecompiler-rewrite-existing-compile-engine-to-match-updated-compiler-flow/9233.
It replaces the Compile_engine with TE_compiler.
@jroesch @YuchenJin
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#issuecomment-894742909
Hey @mikepapadim @YuchenJin what's the status of this PR? Shall we move forward?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on a change in pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#discussion_r684135783
##########
File path: src/relay/backend/te_compiler.cc
##########
@@ -349,6 +350,9 @@ class LowerTensorExpr : public ExprMutator {
Map<GlobalVar, tir::PrimFunc> prim_fns;
for (auto prim_fn : ext_func->funcs->functions) {
+ if (prim_fn.second->GetAttr<String>(attr::kCompiler).defined()) {
Review comment:
Hi @YuchenJin, thanks for getting back to me, the reason I asked was because this loop is gated by the `kCompiler` attribute:
```
if (func->GetAttr<String>(attr::kCompiler).defined()) {
```
And then when it goes into `LowerInternal` it'll end up in this block (with your alterations):
```
if (key->source_func->GetAttr<String>(attr::kCompiler).defined()) {
auto ir_module = IRModule();
const auto name_node = key->source_func->GetAttr<String>(tvm::attr::kGlobalSymbol);
ICHECK(name_node.defined()) << "External function has not been attached a name yet.";
auto func_name = GetUniqueName(name_node.value(), &name_map_);
auto target = Target("ext_dev");
auto global_var = GlobalVar(func_name);
global_var->checked_type_ = key->source_func->checked_type();
ir_module->Add(global_var, key->source_func);
value->cached_func = CachedFunc(target, global_var, {}, {}, te::Schedule(), {}, ir_module);
return value;
}
```
Which means when we enter this loop, the only function in the `IRModule` should be a function taken from `key->source_func` which has the attr on it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on a change in pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#discussion_r684769685
##########
File path: src/relay/backend/te_compiler.cc
##########
@@ -349,6 +350,9 @@ class LowerTensorExpr : public ExprMutator {
Map<GlobalVar, tir::PrimFunc> prim_fns;
for (auto prim_fn : ext_func->funcs->functions) {
+ if (prim_fn.second->GetAttr<String>(attr::kCompiler).defined()) {
Review comment:
Let's try removing the loop, if tests still pass we should be confident it wasn't necessary :smile_cat:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] YuchenJin commented on pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
YuchenJin commented on pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#issuecomment-893590873
As a side note, this PR just naively replaces the old `CompileEngine` in the VM compiler with the lowering functions in the new `TECompiler`. This is the first step. Ultimately, we want to invoke the `LowerTE` pass once in `TECompiler` and hide those lowering functions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on a change in pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#discussion_r683356731
##########
File path: src/relay/backend/te_compiler.cc
##########
@@ -349,6 +350,9 @@ class LowerTensorExpr : public ExprMutator {
Map<GlobalVar, tir::PrimFunc> prim_fns;
for (auto prim_fn : ext_func->funcs->functions) {
+ if (prim_fn.second->GetAttr<String>(attr::kCompiler).defined()) {
Review comment:
Just querying, when is this untrue for an external function that's gone via `Lower` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#issuecomment-895043118
Thank you guys for the effort!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 merged pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
junrushao1994 merged pull request #8501:
URL: https://github.com/apache/tvm/pull/8501
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] YuchenJin commented on a change in pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
YuchenJin commented on a change in pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#discussion_r684812761
##########
File path: src/relay/backend/te_compiler.cc
##########
@@ -349,6 +350,9 @@ class LowerTensorExpr : public ExprMutator {
Map<GlobalVar, tir::PrimFunc> prim_fns;
for (auto prim_fn : ext_func->funcs->functions) {
+ if (prim_fn.second->GetAttr<String>(attr::kCompiler).defined()) {
Review comment:
Yeah, tests still pass after removing it. @jroesch, could you review this PR? :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] YuchenJin commented on a change in pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
YuchenJin commented on a change in pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#discussion_r683599565
##########
File path: src/relay/backend/te_compiler.cc
##########
@@ -349,6 +350,9 @@ class LowerTensorExpr : public ExprMutator {
Map<GlobalVar, tir::PrimFunc> prim_fns;
for (auto prim_fn : ext_func->funcs->functions) {
+ if (prim_fn.second->GetAttr<String>(attr::kCompiler).defined()) {
Review comment:
Hi @Mousius, After calling `TECompilerImpl::Lower()` on an external function, the TECompiler will just encapsulate it into `CachedFunc` and return, and the external function is not lowered. It will be lowered by the external codegen with the `TECompilerNode::LowerExternalFunctions()` later. I added a line `ir_module->Add(global_var, key->source_func);` to `LowerInternal`, so this is untrue when some function in the `funcs` of the `CachedFuncNode` is not lowered yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] YuchenJin commented on a change in pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
YuchenJin commented on a change in pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#discussion_r684546402
##########
File path: src/relay/backend/te_compiler.cc
##########
@@ -349,6 +350,9 @@ class LowerTensorExpr : public ExprMutator {
Map<GlobalVar, tir::PrimFunc> prim_fns;
for (auto prim_fn : ext_func->funcs->functions) {
+ if (prim_fn.second->GetAttr<String>(attr::kCompiler).defined()) {
Review comment:
Thanks @Mousius for elaborating it! I think you are right, and we can probably remove this loop because the `IRModule` should not contain a PrimFunc anyway for external functions, and we don't need to check it. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mikepapadim commented on pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
mikepapadim commented on pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#issuecomment-893328733
@jroesch can this merged?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mikepapadim commented on pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
mikepapadim commented on pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#issuecomment-893328733
@jroesch can this merged?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] YuchenJin commented on pull request #8501: [Relay] Replace compile engine with TE compiler in the VM
Posted by GitBox <gi...@apache.org>.
YuchenJin commented on pull request #8501:
URL: https://github.com/apache/tvm/pull/8501#issuecomment-894752447
> Hey @mikepapadim @YuchenJin what's the status of this PR? Shall we move forward?
Hey @junrushao1994, thanks for taking a look! @Mousius asked a good question and I think we can probably improve the code a little bit. Let's wait @Mousius for his reply. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org