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/10/19 18:40:59 UTC
[GitHub] [tvm] mbs-octoml opened a new pull request #9326: [DRAFT] Switch device planning to be in units of SEScope
mbs-octoml opened a new pull request #9326:
URL: https://github.com/apache/tvm/pull/9326
NOT READY FOR REVIEW
Continuing on from #9313, as part of https://github.com/apache/tvm-rfcs/pull/38, we switch plan_devices.cc to yield SEScopes instead of DLDeviceTypes.
- "on_device" and "device_copy" attributes use SEScopes. Python bindings still use Devices so the only observable change will be that device_ids in annotations now flow all the way through to codegen. We can extend these bindings to allow an exact target to be given in the future should we outgrow the existing 'device type implies unique target' convention.
- Move responsibility for resolving device types to targets into device planner and out of LowerTEPass. All that does is make sure every SEScope entering the system via an annotation ends up with a target if it doesn't already have one.
- Update VM, AOT, Interpreter and Graph executors to use SEScopes on memory operators instead of just device type or Device.
- Centralize all device defaulting logic to use the CompilerConfig helper class.
--
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 #9326: Switch PlanDevices pass to be w.r.t. SEScopes instead of DLDeviceTypes.
Posted by GitBox <gi...@apache.org>.
junrushao1994 merged pull request #9326:
URL: https://github.com/apache/tvm/pull/9326
--
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] mbs-octoml commented on pull request #9326: Switch PlanDevices pass to be w.r.t. SEScopes instead of DLDeviceTypes.
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#issuecomment-966357029
I'm trying to repro the tests/python/driver/tvmc/test_compiler.py::test_compile_tflite_module_with_external_codegen_cmsisnn failure.
Note https://github.com/apache/tvm/pull/9480, which changed the assert for 3 to 4 artifacts, while I'm still getting 3. That's suspicions.
Attempting to repro in the docker ci-cpu image terminates my remote desktop session (?!?). I'll try to get the cnsis-nn setup going directly, which is a good chance to experience the BYOC dependency setup first hand.
--
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] jroesch commented on a change in pull request #9326: Switch PlanDevices pass to be w.r.t. SEScopes instead of DLDeviceTypes.
Posted by GitBox <gi...@apache.org>.
jroesch commented on a change in pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#discussion_r746297164
##########
File path: src/relay/backend/te_compiler.h
##########
@@ -173,31 +173,28 @@ Map<Target, IRModule> GetPerTargetModules(IRModule mod);
* to TE expressions, schedules them, and then to TIR.
*
* \param module The IRModule.
- * \param targets The mapping for devices to targets.
* \param memory_plan The memory plan used during lowering
* \param module_name The name of this module
* \param process_fn Callback allowing one-level up code generators to process
* each function that we lower
* \return The lowered module, see above.
*/
IRModule LowerTE(
- const IRModule& module, TargetMap targets, backend::StaticMemoryPlan memory_plan,
- const String& module_name, ProcessFn process_fn = [](Function f) {});
+ const IRModule& module, backend::StaticMemoryPlan memory_plan, const String& module_name,
Review comment:
Do we have a tracking issue for pulling out the mangling from the lowering? probably makes sense after your most recent draft change?
##########
File path: tests/python/unittest/test_micro_model_library_format.py
##########
@@ -411,4 +410,7 @@ def test_export_byoc_c_module():
if __name__ == "__main__":
- sys.exit(pytest.main([__file__] + sys.argv[1:]))
+ import sys
+
+ # sys.exit(pytest.main([__file__] + sys.argv[1:]))
+ test_export_operator_model_library_format()
Review comment:
We should probably fix this, feel free to do in follow up.
--
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] mbs-octoml commented on pull request #9326: [DRAFT] Switch device planning to be in units of SEScope
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#issuecomment-961304592
(Nearly ready for review -- using ci to tease out what i've broken)
--
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] mbs-octoml commented on pull request #9326: [DRAFT] Switch device planning to be in units of SEScope
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#issuecomment-961304592
(Nearly ready for review -- using ci to tease out what i've broken)
--
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] mbs-octoml commented on a change in pull request #9326: Switch PlanDevices pass to be w.r.t. SEScopes instead of DLDeviceTypes.
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on a change in pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#discussion_r746725199
##########
File path: src/relay/backend/build_module.cc
##########
@@ -207,7 +210,7 @@ class RelayBuildModule : public runtime::ModuleNode {
} else if (name == "optimize") {
return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) {
ICHECK_EQ(args.num_args, 2);
- *rv = this->Optimize(args[0], args[1], this->params_);
Review comment:
The code was using both local and member params simultaneousy, but the local was only ever bound to the member anyway. So I just got rid of the local.
Note there's some weirdness in this API as to what state is expected to come over between 'build' and 'optimize'.
--
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] mbs-octoml commented on a change in pull request #9326: Switch PlanDevices pass to be w.r.t. SEScopes instead of DLDeviceTypes.
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on a change in pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#discussion_r746732749
##########
File path: tests/python/unittest/test_micro_model_library_format.py
##########
@@ -411,4 +410,7 @@ def test_export_byoc_c_module():
if __name__ == "__main__":
- sys.exit(pytest.main([__file__] + sys.argv[1:]))
+ import sys
+
+ # sys.exit(pytest.main([__file__] + sys.argv[1:]))
+ test_export_operator_model_library_format()
Review comment:
Woops, thanks.
--
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] mbs-octoml commented on a change in pull request #9326: Switch PlanDevices pass to be w.r.t. SEScopes instead of DLDeviceTypes.
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on a change in pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#discussion_r746732515
##########
File path: src/relay/backend/te_compiler.h
##########
@@ -173,31 +173,28 @@ Map<Target, IRModule> GetPerTargetModules(IRModule mod);
* to TE expressions, schedules them, and then to TIR.
*
* \param module The IRModule.
- * \param targets The mapping for devices to targets.
* \param memory_plan The memory plan used during lowering
* \param module_name The name of this module
* \param process_fn Callback allowing one-level up code generators to process
* each function that we lower
* \return The lowered module, see above.
*/
IRModule LowerTE(
- const IRModule& module, TargetMap targets, backend::StaticMemoryPlan memory_plan,
- const String& module_name, ProcessFn process_fn = [](Function f) {});
+ const IRModule& module, backend::StaticMemoryPlan memory_plan, const String& module_name,
Review comment:
That's CORE-47 I believe.
--
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] mbs-octoml commented on pull request #9326: [DRAFT] Switch device planning to be in units of SEScope
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#issuecomment-961304592
(Nearly ready for review -- using ci to tease out what i've broken)
--
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] mbs-octoml commented on a change in pull request #9326: Switch PlanDevices pass to be w.r.t. SEScopes instead of DLDeviceTypes.
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on a change in pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#discussion_r746732749
##########
File path: tests/python/unittest/test_micro_model_library_format.py
##########
@@ -411,4 +410,7 @@ def test_export_byoc_c_module():
if __name__ == "__main__":
- sys.exit(pytest.main([__file__] + sys.argv[1:]))
+ import sys
+
+ # sys.exit(pytest.main([__file__] + sys.argv[1:]))
+ test_export_operator_model_library_format()
Review comment:
Woops, thanks. I need to go around again anyway since I somehow lost the SplitArgs xform (and CI didn't notice...)
--
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] jroesch commented on a change in pull request #9326: Switch PlanDevices pass to be w.r.t. SEScopes instead of DLDeviceTypes.
Posted by GitBox <gi...@apache.org>.
jroesch commented on a change in pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#discussion_r746289994
##########
File path: src/relay/backend/build_module.cc
##########
@@ -207,7 +210,7 @@ class RelayBuildModule : public runtime::ModuleNode {
} else if (name == "optimize") {
return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) {
ICHECK_EQ(args.num_args, 2);
- *rv = this->Optimize(args[0], args[1], this->params_);
Review comment:
Why do we no longer need the parameters 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] mbs-octoml commented on pull request #9326: Switch PlanDevices pass to be w.r.t. SEScopes instead of DLDeviceTypes.
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on pull request #9326:
URL: https://github.com/apache/tvm/pull/9326#issuecomment-967147706
Ready to merge.
--
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