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 2022/12/13 11:13:18 UTC
[GitHub] [tvm] masahi opened a new pull request, #13606: [Relay][TIR] Add utility to lower Relay func to TIR prim func
masahi opened a new pull request, #13606:
URL: https://github.com/apache/tvm/pull/13606
This is very useful for creating a TensorIR program that is otherwise hard to so via manual TVMScript or te compute + `create_prim_func`.
--
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 pull request #13606: [Relay][TIR] Add utility to lower Relay func to TIR prim func
Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #13606:
URL: https://github.com/apache/tvm/pull/13606#issuecomment-1352929237
> @Mousius The new function simply composes the existing interface to the TECompiler, `LowerTECompute` and `CreatePrimFunc` that converts lowered TE compute to TensorIR. `LowerTECompute` was introduced after the compile engine refactor you mentioned completed, and made only possible thanks to that refactoring work. So I don't get your concern here.
I find this equally concerning, given this is also exposing more of the internals of the TE Compiler rather than having clear boundaries within the codebase.
> What I wanted to add is a simple interface that converts one Relay primitive function to the corresponding TIR prim func. Then I realized that backend/task_extraction.cc already has code that does exactly that, so I simply extracted into a reusable function and exposed it to Python. We require the input to be a primitive function, so we never generates multiple TIR prim funcs.
Exposing it python and as part of the public API means others can access it, which means that we have to support it as a way of using the compiler rather than as an aspect of meta-scheduler. This is similar to the previous methods that we were unable to deprecate in https://github.com/apache/tvm/pull/13606.
> This is just a convention of MetaSchedule, storing an IRModule with one prim func as a task. I didn't change any behavior about task extraction in this PR. What we want is a `relay::Function -> tir::PrimFunc` interface, `IRModule -> IRModule` is valid but it wouldn't convey what this new API is meant for. `LowerTE` is also overkill since I don't need to lower schedules and I'm not concerned about external codegen etc.
I don't think `LowerTE` is overkill in practice, if it doesn't find any external codegen or other things to do it won't take any action - it's a standardised interface between two modules in the codebase, you've already indicated meta schedule is breaking that boundary and that leads us back to the original issues with CompileEngine.
--
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 pull request #13606: [Relay][TIR] Add utility to lower Relay func to TIR prim func
Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #13606:
URL: https://github.com/apache/tvm/pull/13606#issuecomment-1351632169
:wave: Hello @masahi / @csullivan / @echuraev / @junrushao,
Whilst I appreciate this is useful with the use-case demonstrated, I believe it might've needed a bit more time for others to formulate a review as it was up for less than 24 hours before merge?
One of my concerns is that this seems to be creating new public interfaces to the internals of the TECompiler which in the past we've attempted to remove:
* https://discuss.tvm.apache.org/t/rfc-relay-tecompiler-rewrite-existing-compile-engine-to-match-updated-compiler-flow/9233
* https://github.com/apache/tvm/pull/8775
* https://github.com/apache/tvm/pull/9282
This change means we have to maintain an interface which generates a single `tir::PrimFunc` from a `relay::Function` where-as it could potentially lead to multiple `tir::PrimFunc` with a single `IRModule` as a result. The next line down:
```
IRModule tir_mod = PrimFuncToIRModule(f.value());
```
Implies we wanted it in `IRModule` form anyway, so creating an `IRModule` with a `Function` and running `LowerTE(mod)` seems the best result here?
--
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] csullivan commented on a diff in pull request #13606: [Relay][TIR] Add utility to lower Relay func to TIR prim func
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #13606:
URL: https://github.com/apache/tvm/pull/13606#discussion_r1048001996
##########
src/relay/backend/te_compiler_cache.cc:
##########
@@ -1088,6 +1088,30 @@ std::tuple<Array<te::Tensor>, Array<runtime::NDArray>, std::string> LowerTECompu
return std::make_tuple(tensor_outs, constants, lower_te_compute.candidate_name_);
}
+std::pair<Optional<tir::PrimFunc>, std::string> LowerToPrimFunc(const Function& relay_func,
Review Comment:
Should we add a check in here that relay_func has attr::kPrimitive to alert the user early if they are trying to lower a non-primitive function?
--
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] masahi commented on a diff in pull request #13606: [Relay][TIR] Add utility to lower Relay func to TIR prim func
Posted by GitBox <gi...@apache.org>.
masahi commented on code in PR #13606:
URL: https://github.com/apache/tvm/pull/13606#discussion_r1048018778
##########
src/relay/backend/te_compiler_cache.cc:
##########
@@ -1088,6 +1088,30 @@ std::tuple<Array<te::Tensor>, Array<runtime::NDArray>, std::string> LowerTECompu
return std::make_tuple(tensor_outs, constants, lower_te_compute.candidate_name_);
}
+std::pair<Optional<tir::PrimFunc>, std::string> LowerToPrimFunc(const Function& relay_func,
Review Comment:
Thanks, yes we should do that. Updated.
--
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] masahi commented on pull request #13606: [Relay][TIR] Add utility to lower Relay func to TIR prim func
Posted by GitBox <gi...@apache.org>.
masahi commented on PR #13606:
URL: https://github.com/apache/tvm/pull/13606#issuecomment-1352236597
@Mousius The new function simply composes the existing interface to the TECompiler, `LowerTECompute` and `CreatePrimFunc` that converts lowered TE compute to TensorIR. `LowerTECompute` was introduced after the compile engine refactor you mentioned completed, and made only possible thanks to that refactoring work. So I don't get your concern here.
What I wanted to add is a simple interface that converts one Relay primitive function to the corresponding TIR prim func. Then I realized that `backend/task_extraction.cc` already has code that does exactly that, so I simply extracted into a reusable function and exposed it to Python. We require the input to be a primitive function, so we never generates multiple TIR prim funcs.
> Implies we wanted it in IRModule form anyway, so creating an IRModule with a Function and running LowerTE(mod) seems the best result here?
This is just MetaSchedule storing an IRModule in a task. I didn't change any behavior about task extraction in this PR. What we want is a `relay::Function -> tir::PrimFunc` interface, `IRModule -> IRModule` is valid but it wouldn't convey what this new API is meant for.
--
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] tvm-bot commented on pull request #13606: [Relay][TIR] Add utility to lower Relay func to TIR prim func
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13606:
URL: https://github.com/apache/tvm/pull/13606#issuecomment-1348219783
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @Hzfengsy, @junrushao <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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] masahi merged pull request #13606: [Relay][TIR] Add utility to lower Relay func to TIR prim func
Posted by GitBox <gi...@apache.org>.
masahi merged PR #13606:
URL: https://github.com/apache/tvm/pull/13606
--
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