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/29 09:11:22 UTC
[GitHub] [tvm] Mousius opened a new pull request #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Mousius opened a new pull request #9395:
URL: https://github.com/apache/tvm/pull/9395
This patch adds support for passing the device context via the unpacked API in AOT, generating an additional struct if necessary:
```c
/*!
* \brief Device context pointers for TVM module "default"
*/
struct tvmgen_default_devices {
void* npu;
};
```
Which is then added as an argument to the entry function:
```c
/*!
* \brief entrypoint function for TVM module "default"
* \param inputs Input tensors for the module
* \param outputs Output tensors for the module
* \param devices Device context pointers for the module
*/
int32_t tvmgen_default_run(
struct tvmgen_default_inputs* inputs,
struct tvmgen_default_outputs* outputs,
struct tvmgen_default_devices* devices
);
```
I've temporarily added the collection of external code generators to the TE compiler pending proper annotation of the eventual functions.
Co-authored-by: Grant Watson <gr...@arm.com>
--
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] areusch commented on a change in pull request #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r745233508
##########
File path: tests/cpp/target/source/interface_c_test.cc
##########
@@ -69,7 +69,29 @@ TEST(InterfaceAPI, ContainsRunFunction) {
<< " struct tvmgen_ultimate_cat_spotter_outputs* outputs\n"
<< ");\n";
- runtime::Module test_module = InterfaceCCreate("ultimate_cat_spotter", {"input"}, {"output"});
+ runtime::Module test_module = InterfaceCCreate("ultimate_cat_spotter", {"input"}, {"output"}, {});
+ std::string header_source = test_module->GetSource();
+
+ ASSERT_THAT(header_source, HasSubstr(run_function.str()));
+}
+
+TEST(InterfaceAPI, ContainsRunFunctionWithDevices) {
+ std::stringstream run_function;
+
+ run_function << "/*!\n"
+ << " * \\brief entrypoint function for TVM module \"ultimate_cat_spotter\"\n"
+ << " * \\param inputs Input tensors for the module \n"
Review comment:
should "ultimate_cat_spotter" be repeated? :)
##########
File path: src/target/target_kind.cc
##########
@@ -373,6 +373,8 @@ TVM_REGISTER_TARGET_KIND("hybrid", kDLCPU) // line break
TVM_REGISTER_TARGET_KIND("composite", kDLCPU).add_attr_option<Array<Target>>("devices");
+TVM_REGISTER_TARGET_KIND("ethos-u", kDLCPU).set_attr<Bool>("c_device_api", Bool(true));
Review comment:
what about a target like Ethos-N, which could in theory be used from either C or C++ runtimes? should we just call this attribute `use_device_api`?
##########
File path: apps/microtvm/ethosu/src/tvm_ethosu_runtime.c
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "tvm_ethosu_runtime.h"
+
+#include <ethosu_driver.h>
+
+int32_t TVMEthosULaunch(struct ethosu_driver* driver, void* cms_data, size_t cms_data_size,
+ uint64_t* base_addrs, size_t* base_addrs_size, int num_tensors) {
+ int32_t result =
+ ethosu_invoke(driver, cms_data, cms_data_size, base_addrs, base_addrs_size, num_tensors);
+
+ // Map errors in invoke to TVM errors
+ if (result != 0) {
+ return -1;
Review comment:
does it make sense to just return the error code here? e.g. curious if the above comment is a TODO or a descriptor
--
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] areusch commented on a change in pull request #9395: [1/3][AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r748561136
##########
File path: apps/microtvm/ethosu/src/tvm_ethosu_runtime.c
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "tvm_ethosu_runtime.h"
+
+#include <ethosu_driver.h>
+
+int32_t TVMEthosULaunch(struct ethosu_driver* driver, void* cms_data, size_t cms_data_size,
+ uint64_t* base_addrs, size_t* base_addrs_size, int num_tensors) {
+ int32_t result =
+ ethosu_invoke(driver, cms_data, cms_data_size, base_addrs, base_addrs_size, num_tensors);
+
+ // Map errors in invoke to TVM errors
+ if (result != 0) {
+ return -1;
Review comment:
we don't need to fix this in this PR, but i'm a big believer in not squashing error codes if at all possible. almost every mystery i've ever experienced in an embedded system has been the kind of problem where more data almost always is the most effective way to understand/solve the problem. we can squash for now but it'd be good to think about ways to expose driver error codes. that could be us allocating a new error code in error_codes.h and then stashing additional information somewhere else (perhaps that calls for yet another pointer to passed in? or perhaps the implementation should just be left to the driver and the driver struct should be expected to carry 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 pull request #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#issuecomment-954581727
@areusch I've done the bare minimum to wire this through here, and want to follow up later with the `cpacked` variant that was actually working pretty well from just variables rather than adding the `resource_handle` attribute.
--
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 #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r745463623
##########
File path: tests/cpp/target/source/interface_c_test.cc
##########
@@ -69,7 +69,29 @@ TEST(InterfaceAPI, ContainsRunFunction) {
<< " struct tvmgen_ultimate_cat_spotter_outputs* outputs\n"
<< ");\n";
- runtime::Module test_module = InterfaceCCreate("ultimate_cat_spotter", {"input"}, {"output"});
+ runtime::Module test_module = InterfaceCCreate("ultimate_cat_spotter", {"input"}, {"output"}, {});
+ std::string header_source = test_module->GetSource();
+
+ ASSERT_THAT(header_source, HasSubstr(run_function.str()));
+}
+
+TEST(InterfaceAPI, ContainsRunFunctionWithDevices) {
+ std::stringstream run_function;
+
+ run_function << "/*!\n"
+ << " * \\brief entrypoint function for TVM module \"ultimate_cat_spotter\"\n"
+ << " * \\param inputs Input tensors for the module \n"
Review comment:
Personally, I think it's neater to define the specific module at the top of the `\brief` and then reference it from the params, so if you generate docs you get all the information for the struct without too much repetition. As that's the current behaviour of the generator, if we do want to make changes to the copy it'll introduce a lot of noise so we should do it in a separate PR :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] mbs-octoml commented on pull request #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#issuecomment-966437670
LGTM from me.
--
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] areusch commented on a change in pull request #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r746796926
##########
File path: src/target/target_kind.cc
##########
@@ -373,6 +373,8 @@ TVM_REGISTER_TARGET_KIND("hybrid", kDLCPU) // line break
TVM_REGISTER_TARGET_KIND("composite", kDLCPU).add_attr_option<Array<Target>>("devices");
+TVM_REGISTER_TARGET_KIND("ethos-u", kDLCPU).set_attr<Bool>("c_device_api", Bool(true));
Review comment:
aha so :) we are going to need to find a way to encode Device API calls in TIR which is agnostic to the runtime, i think. There is a way to get the `Module` instance in C++ runtime given a TVMDevice.
--
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] areusch merged pull request #9395: [1/3][AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
areusch merged pull request #9395:
URL: https://github.com/apache/tvm/pull/9395
--
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 #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r746805658
##########
File path: apps/microtvm/ethosu/src/tvm_ethosu_runtime.c
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "tvm_ethosu_runtime.h"
+
+#include <ethosu_driver.h>
+
+int32_t TVMEthosULaunch(struct ethosu_driver* driver, void* cms_data, size_t cms_data_size,
+ uint64_t* base_addrs, size_t* base_addrs_size, int num_tensors) {
+ int32_t result =
+ ethosu_invoke(driver, cms_data, cms_data_size, base_addrs, base_addrs_size, num_tensors);
+
+ // Map errors in invoke to TVM errors
+ if (result != 0) {
+ return -1;
Review comment:
Ah, the intention was to document that I was mapping the error explicitly as it's a bit odd otherwise, hopefully we'll have a better example where the values are more interesting at some point.
On the speculative bit, I think operator errors work differently from runtime errors in this case? Mostly because the executor only really needs to know if the operator failed or not, so I think `0` and `-1` are sufficient for that use case rather than adding the rest of the runtime error codes?
--
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 #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r745457411
##########
File path: apps/microtvm/ethosu/src/tvm_ethosu_runtime.c
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "tvm_ethosu_runtime.h"
+
+#include <ethosu_driver.h>
+
+int32_t TVMEthosULaunch(struct ethosu_driver* driver, void* cms_data, size_t cms_data_size,
+ uint64_t* base_addrs, size_t* base_addrs_size, int num_tensors) {
+ int32_t result =
+ ethosu_invoke(driver, cms_data, cms_data_size, base_addrs, base_addrs_size, num_tensors);
+
+ // Map errors in invoke to TVM errors
+ if (result != 0) {
+ return -1;
Review comment:
Are you referring to the CRT `kError` constants? I was keeping it runtime-agnostic and mapping it similar to the normal error handling in operators:
https://github.com/apache/tvm/blob/7c3283cb8a5da199d698f435754b24181c0322d2/src/target/source/codegen_c_host.cc#L253-L276
I don't think that's the shared CRT/C++ approach?
--
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] areusch commented on a change in pull request #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r746795050
##########
File path: apps/microtvm/ethosu/src/tvm_ethosu_runtime.c
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "tvm_ethosu_runtime.h"
+
+#include <ethosu_driver.h>
+
+int32_t TVMEthosULaunch(struct ethosu_driver* driver, void* cms_data, size_t cms_data_size,
+ uint64_t* base_addrs, size_t* base_addrs_size, int num_tensors) {
+ int32_t result =
+ ethosu_invoke(driver, cms_data, cms_data_size, base_addrs, base_addrs_size, num_tensors);
+
+ // Map errors in invoke to TVM errors
+ if (result != 0) {
+ return -1;
Review comment:
right, i think adopting the != 0 convention is fine here. just was curious whether you had a TODO to try and map into the error space or if the comment was descriptive and you wanted to keep as-is. sounds like the latter, which is fine
one thing i had semi-speculatively adopted was: if the top bit is 1, the error is a system-specific thing. curious if that is helpful here? while i don't think we can exactly work around this, one misleading thing would be to return an error code which is in error_codes.h, but is actually specific to this device API.
--
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 #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r746793358
##########
File path: src/relay/backend/aot_executor_codegen.cc
##########
@@ -637,6 +681,7 @@ class AOTExecutorCodegen : public MixedModeVisitor {
main_signature_.push_back(tir::Var("output", DataType::Handle()));
}
+ CollectDeviceVariables(lowered_mod->GetAttr<Map<GlobalVar, String>>("device_contexts").value());
Review comment:
I think the blocker for pulling the device (or, after #9326 the SEScope) for the callee is just that we're not carrying those annotations over into the PrimFuncs, right? Would you consider trying that after #9326? I've unfortunately hit my limit for any more hackery within the te_compiler.cc.
Then, afaikt you're doing what I've also had to do in the VM compiller.cc, which is allocate globally unique tags (just ints in the VM) for every unique SEScope, and build a global map from those tags to their actual devices (just an array of Devices in the VM). Everything you've done to achieve that part looks perfectly sensible to me.
--
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 #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r746830667
##########
File path: src/relay/backend/aot_executor_codegen.cc
##########
@@ -637,6 +681,7 @@ class AOTExecutorCodegen : public MixedModeVisitor {
main_signature_.push_back(tir::Var("output", DataType::Handle()));
}
+ CollectDeviceVariables(lowered_mod->GetAttr<Map<GlobalVar, String>>("device_contexts").value());
Review comment:
The Call gets translated to `Call(GlobalVar)`, the `PrimFunc` is just a stub one for the TE Compiler and because the function name may change I can't find the eventual `Function` again. Even if I could guarantee the `GlobalVar` didn't change, and find the `Function`, the annotation gets stripped by `LowerExternalFunctions`. When this gets revisited the aim would be to not remove the annotation, so it can be found again later, I believe it's done now because `kCompiler` causes different paths in different parts of the compiler which are undesirable.
--
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 #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r746824449
##########
File path: src/relay/backend/aot_executor_codegen.cc
##########
@@ -637,6 +681,7 @@ class AOTExecutorCodegen : public MixedModeVisitor {
main_signature_.push_back(tir::Var("output", DataType::Handle()));
}
+ CollectDeviceVariables(lowered_mod->GetAttr<Map<GlobalVar, String>>("device_contexts").value());
Review comment:
> Internally to the TECompiler, the representation of the BYOC function gets changed to a GlobalVar after the name gets uniquified, and the annotation gets removed, so until everything gets persisted naturally all the way down I can't get to it sensibly without this
Hmm, ok, so you're still on the BYOC compilation notion of 'target' while I'm on the, well, Target notion of 'target'.
Even so, you can avoid the IRModule attribute map by annotating the PrimFunc, right? That's at least closer to the ultimate goal.
--
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 #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r745466545
##########
File path: src/target/target_kind.cc
##########
@@ -373,6 +373,8 @@ TVM_REGISTER_TARGET_KIND("hybrid", kDLCPU) // line break
TVM_REGISTER_TARGET_KIND("composite", kDLCPU).add_attr_option<Array<Target>>("devices");
+TVM_REGISTER_TARGET_KIND("ethos-u", kDLCPU).set_attr<Bool>("c_device_api", Bool(true));
Review comment:
I think my nervousness here stems from enabling the Device API when the runtime doesn't support it. Given the C++ runtime uses the `Module` nodes rather than C functions, `Target`s could enumerate support rather than enabling all Device APIs?
I'm happy to change this to `use_device_api` to match the RFC as I think there's bigger problems with enumerating the internal support for runtimes and executors but curious on your thoughts?
--
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] areusch commented on a change in pull request #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r746797166
##########
File path: tests/cpp/target/source/interface_c_test.cc
##########
@@ -69,7 +69,29 @@ TEST(InterfaceAPI, ContainsRunFunction) {
<< " struct tvmgen_ultimate_cat_spotter_outputs* outputs\n"
<< ");\n";
- runtime::Module test_module = InterfaceCCreate("ultimate_cat_spotter", {"input"}, {"output"});
+ runtime::Module test_module = InterfaceCCreate("ultimate_cat_spotter", {"input"}, {"output"}, {});
+ std::string header_source = test_module->GetSource();
+
+ ASSERT_THAT(header_source, HasSubstr(run_function.str()));
+}
+
+TEST(InterfaceAPI, ContainsRunFunctionWithDevices) {
+ std::stringstream run_function;
+
+ run_function << "/*!\n"
+ << " * \\brief entrypoint function for TVM module \"ultimate_cat_spotter\"\n"
+ << " * \\param inputs Input tensors for the module \n"
Review comment:
ok that seems great to me.
--
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 #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r747640233
##########
File path: src/relay/backend/aot_executor_codegen.cc
##########
@@ -637,6 +681,7 @@ class AOTExecutorCodegen : public MixedModeVisitor {
main_signature_.push_back(tir::Var("output", DataType::Handle()));
}
+ CollectDeviceVariables(lowered_mod->GetAttr<Map<GlobalVar, String>>("device_contexts").value());
Review comment:
Ack. Thanks for the extra comment.
--
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 #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r745457411
##########
File path: apps/microtvm/ethosu/src/tvm_ethosu_runtime.c
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "tvm_ethosu_runtime.h"
+
+#include <ethosu_driver.h>
+
+int32_t TVMEthosULaunch(struct ethosu_driver* driver, void* cms_data, size_t cms_data_size,
+ uint64_t* base_addrs, size_t* base_addrs_size, int num_tensors) {
+ int32_t result =
+ ethosu_invoke(driver, cms_data, cms_data_size, base_addrs, base_addrs_size, num_tensors);
+
+ // Map errors in invoke to TVM errors
+ if (result != 0) {
+ return -1;
Review comment:
Are you referring to the CRT `kError` constants? I was keeping it runtime-agnostic and mapping it similar to the normal error handling in operators:
https://github.com/apache/tvm/blob/7c3283cb8a5da199d698f435754b24181c0322d2/src/target/source/codegen_c_host.cc#L253-L276
I think that's the shared CRT/C++ approach?
--
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] areusch commented on a change in pull request #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r746795050
##########
File path: apps/microtvm/ethosu/src/tvm_ethosu_runtime.c
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "tvm_ethosu_runtime.h"
+
+#include <ethosu_driver.h>
+
+int32_t TVMEthosULaunch(struct ethosu_driver* driver, void* cms_data, size_t cms_data_size,
+ uint64_t* base_addrs, size_t* base_addrs_size, int num_tensors) {
+ int32_t result =
+ ethosu_invoke(driver, cms_data, cms_data_size, base_addrs, base_addrs_size, num_tensors);
+
+ // Map errors in invoke to TVM errors
+ if (result != 0) {
+ return -1;
Review comment:
right, i think adopting the != 0 convention is fine here. just was curious whether you had a TODO to try and map into the error space or if the comment was descriptive and you wanted to keep as-is. sounds like the latter, which is fine
one thing i had semi-speculatively adopted was: if the top bit is 1, the error is a system-specific thing. curious if that is helpful 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] Mousius commented on a change in pull request #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r746812054
##########
File path: src/relay/backend/aot_executor_codegen.cc
##########
@@ -637,6 +681,7 @@ class AOTExecutorCodegen : public MixedModeVisitor {
main_signature_.push_back(tir::Var("output", DataType::Handle()));
}
+ CollectDeviceVariables(lowered_mod->GetAttr<Map<GlobalVar, String>>("device_contexts").value());
Review comment:
Sorry @mbs-octoml, I feel like I'm justified in just one more hack for this :crying_cat_face:
Unfortunately we also need to rectify the `kCompiler` vs `Target` mix at the same time as this current implementation uses what I'm calling a hybrid `Target` (a registered `Target` which is still using most of the BYOC infrastructure). The eventual aim is to get rid of `kCompiler` and move fully to `Target` so that the above annotations end up on all `Function`s of relevance and then get rid of this.
Internally to the TECompiler, the representation of the BYOC function gets changed to a `GlobalVar` after the name gets uniquified, and the annotation gets removed, so until everything gets persisted naturally all the way down I can't get to it sensibly without this :crying_cat_face:
Or I could re-use some way of getting to the TE Compiler internal cache if you'd like? :smiling_imp:
--
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] areusch commented on pull request #9395: [1/3][AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#issuecomment-967472348
thanks @Mousius @mbs-octoml the PR is now 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] Mousius commented on a change in pull request #9395: [AOT][DeviceAPI] Connecting devices structure to relevant operators
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9395:
URL: https://github.com/apache/tvm/pull/9395#discussion_r745817252
##########
File path: apps/microtvm/ethosu/src/tvm_ethosu_runtime.c
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "tvm_ethosu_runtime.h"
+
+#include <ethosu_driver.h>
+
+int32_t TVMEthosULaunch(struct ethosu_driver* driver, void* cms_data, size_t cms_data_size,
+ uint64_t* base_addrs, size_t* base_addrs_size, int num_tensors) {
+ int32_t result =
+ ethosu_invoke(driver, cms_data, cms_data_size, base_addrs, base_addrs_size, num_tensors);
+
+ // Map errors in invoke to TVM errors
+ if (result != 0) {
+ return -1;
Review comment:
Ah, further to my above explanation, the invoke may return a value which is incompatible with TVM - so that should be catered for here explicitly in the invoke <-> TVM glue. For example, if we had add a success return value of `404` from invoke, then this can equally convert `404` to `0` which is the TVM internal contract.
--
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