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/02/18 18:14:15 UTC

[GitHub] [tvm] mehrdadh opened a new pull request #10311: [Draft][runtime][Hexagon] AOTExecutor implementation for C Codegen

mehrdadh opened a new pull request #10311:
URL: https://github.com/apache/tvm/pull/10311


   This is a follow up PR to https://github.com/apache/tvm/pull/10283 which adds hexagon implementation of AOTExecutor.
   
   This PR stays as draft until we merge https://github.com/apache/tvm/pull/10283.
   
   cc @kparzysz-quic @areusch 


-- 
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 #10311: [Draft][runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#issuecomment-1048475231


   @kparzysz-quic some of these are from the underlying PRs #10283 and #10282 , i'll attempt to address them there and respond here when done.


-- 
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] kparzysz-quic commented on a change in pull request #10311: [runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r819105614



##########
File path: tests/python/contrib/test_hexagon/test_launcher.py
##########
@@ -217,24 +223,23 @@ def test_graph_executor(android_serial_number, tvm_tracker_host, tvm_tracker_por
         lowered.get_lib().save(dso_binary_path)
 
     if not android_serial_number:
-        pytest.skip("Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
+        pytest.skip(msg="Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
 
     rpc_info = {
-      "rpc_tracker_host" : tvm_tracker_host,
-      "rpc_tracker_port" : tvm_tracker_port,
-      "rpc_server_port" : 7070,
+        "rpc_tracker_host": tvm_tracker_host,
+        "rpc_tracker_port": tvm_tracker_port,
+        "rpc_server_port": 7070,
     }
     launcher = HexagonLauncher(serial_number=android_serial_number, rpc_info=rpc_info)
     launcher.upload(dso_binary_path, dso_binary)
     launcher.start_server()
 
     with launcher.start_session() as sess:
         graph_mod = launcher.get_graph_executor(lowered.get_graph_json(), dso_binary, sess)
-        weight_in = np.random.rand(5, 5, 3, 8).astype(dtype=dtype)
-        data_in = np.random.rand(1, 64, 64, 3).astype(dtype=dtype)
-        graph_mod.set_input(weight=weight_in)
-        graph_mod.run(data=data_in)
+        graph_mod.set_input(**params)
+        graph_mod.run(**inputs)
         hexagon_output = graph_mod.get_output(0).numpy()
+        launcher.stop_server()

Review comment:
       The idea about the `with` indent is that it starts a session, and leaving the indent should stop the session.  This isn't really happening right now because there is no way (that I could see) to close the connection by doing something like `rpc.close()` in the `__exit__` function in `Session`.
   According to that, the `stop_server()` should happen after the session has been closed, i.e. outside of the indent.  That is,
   ```
   with launcher.start_session() as sess:
       # run, etc.
   # getting here should terminate sess
   launcher.stop_server()
   ```
   I noticed this in several places in this file.

##########
File path: tests/python/contrib/test_hexagon/test_launcher.py
##########
@@ -50,12 +51,12 @@ def test_add(android_serial_number, tvm_tracker_host, tvm_tracker_port):
     func.save(dso_binary_path)
 
     if not android_serial_number:
-        pytest.skip("Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
+        pytest.skip(msg="Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
 
     rpc_info = {
-      "rpc_tracker_host" : tvm_tracker_host,
-      "rpc_tracker_port" : tvm_tracker_port,
-      "rpc_server_port" : 7070,
+        "rpc_tracker_host": tvm_tracker_host,
+        "rpc_tracker_port": tvm_tracker_port,
+        "rpc_server_port": 7070,

Review comment:
       There is a problem I just realized with the RPC server: there is no way that I know of to gracefully shut it down.  When `stop_server()` terminates the server, this leaves the socket (7070) in a `TIME_WAIT` state.  As a consequence, in the next test, `start_server()` would fail to bind to 7070, which would still be considered "in use", and the test as a whole would fail.
   
   My workaround for that was to use a different server port in each test, i.e. 7070, 7071, etc., at least in the short term, until the connection shutdown can be improved.




-- 
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] mehrdadh commented on a change in pull request #10311: [runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r819088919



##########
File path: python/tvm/contrib/hexagon/hexagon.py
##########
@@ -248,3 +265,44 @@ def transform(func, mod, ctx):
 
 def ir_lower_vtcm_pass():
     return [(3, ir_lower_vtcm())]
+
+
+def create_aot_shared(so_name: Union[str, pathlib.Path], files, hexagon_arch: str, options=None):
+    """Export Hexagon AOT module."""
+    if not os.access(str(HEXAGON_CLANG_PLUS), os.X_OK):
+        raise Exception(
+            'The Clang++ "' + str(HEXAGON_CLANG_PLUS) + '" does not exist or is not executable.'
+        )
+    if not HEXAGON_TOOLCHAIN:
+        raise Exception(
+            " The environment variable HEXAGON_TOOLCHAIN is unset. Please export "
+            + "HEXAGON_TOOLCHAIN in your environment."
+        )
+    if not HEXAGON_SDK_PATH:
+        raise Exception(
+            " The environment variable HEXAGON_SDK_PATH is unset. Please export "
+            + "HEXAGON_SDK_PATH in your environment."
+        )
+
+    tvm_dir = pathlib.Path(os.path.dirname(os.path.realpath(__file__))) / ".." / ".." / ".." / ".."
+    compute_arch = f"compute{hexagon_arch}"
+    compile_options = [
+        f"-I{tvm_dir / 'include'}",
+        f"-I{tvm_dir / '3rdparty' / 'dlpack' / 'include'}",
+        f"-I{tvm_dir / '3rdparty' / 'dmlc-core' / 'include'}",
+        f"-I{pathlib.Path(HEXAGON_SDK_PATH) / 'rtos' / 'qurt' / compute_arch / 'include'/ 'posix'}",
+        f"-I{pathlib.Path(HEXAGON_SDK_PATH) / 'rtos' / 'qurt' / compute_arch / 'include' / 'qurt'}",
+        f"-DDMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>",
+        f"-D_MACH_I32=int",
+    ]
+
+    # For debugging
+    for path in HEXAGON_SDK_INCLUDE_DIRS:
+        compile_options.append(f"-I{str(path)}")
+
+    cross_compile = cc.cross_compiler(
+        compile_func=tvm.get_global_func("tvm.contrib.hexagon.hexagon.hexagon_clang_plus")()

Review comment:
       done.




-- 
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] mehrdadh commented on pull request #10311: [runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
mehrdadh commented on pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#issuecomment-1058388651


   @kparzysz-quic PTAL


-- 
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] kparzysz-quic commented on a change in pull request #10311: [Draft][runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r812346287



##########
File path: include/tvm/runtime/metadata.h
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/metadata.h
+ * \brief Defines types which can be used in Metadata.
+ */
+#ifndef TVM_RUNTIME_METADATA_H_
+#define TVM_RUNTIME_METADATA_H_
+
+#include <inttypes.h>
+#include <tvm/runtime/c_runtime_api.h>
+#include <tvm/runtime/metadata_base.h>
+#include <tvm/support/span.h>
+
+#define TVM_METADATA_VERSION 1
+static const constexpr int64_t kMetadataVersion = TVM_METADATA_VERSION;
+#ifdef __cplusplus

Review comment:
       Unnecessary.  This file will not compile as anything other than C++.

##########
File path: src/runtime/hexagon/hexagon/hexagon_device_api_v2.cc
##########
@@ -82,7 +82,11 @@ void* HexagonDeviceAPIv2::AllocDataSpace(Device dev, size_t nbytes, size_t align
 }
 
 void HexagonDeviceAPIv2::FreeDataSpace(Device dev, void* ptr) {
-  CHECK(TVMDeviceExtType(dev.device_type) == kDLHexagon);
+  bool device = false;
+  if ((TVMDeviceExtType(dev.device_type) == kDLHexagon) || (DLDeviceType(dev.device_type) == kDLCPU)) {

Review comment:
       Why is `kDLCPU` allowed here?

##########
File path: include/tvm/runtime/metadata.h
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/metadata.h
+ * \brief Defines types which can be used in Metadata.
+ */
+#ifndef TVM_RUNTIME_METADATA_H_
+#define TVM_RUNTIME_METADATA_H_
+
+#include <inttypes.h>
+#include <tvm/runtime/c_runtime_api.h>
+#include <tvm/runtime/metadata_base.h>
+#include <tvm/support/span.h>
+
+#define TVM_METADATA_VERSION 1
+static const constexpr int64_t kMetadataVersion = TVM_METADATA_VERSION;
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct TVMMetadata {
+  int64_t version;
+  const struct TVMTensorInfo* inputs;
+  int64_t num_inputs;
+  const struct TVMTensorInfo* outputs;
+  int64_t num_outputs;
+  const char** devices;
+  int64_t num_devices;
+  const char* executor;
+  const char* mod_name;
+  const char* interface_api;
+  bool use_unpacked_api;
+};
+
+struct TVMTensorInfo {
+  const char* name;
+  const int64_t* shape;
+  int64_t num_shape;
+  DLDataType dtype;
+};
+#ifdef __cplusplus

Review comment:
       Same.

##########
File path: src/relay/backend/aot_executor_codegen.cc
##########
@@ -819,11 +854,29 @@ class AOTExecutorCodegen : public MixedModeVisitor {
     ICHECK(target_host_.defined()) << "require a target_host to be given for AOT codegen";
     VLOG(1) << "target host: " << target_host_->ToDebugString();
 
+    Runtime runtime_config = mod->GetAttr<Runtime>(tvm::attr::kRuntime).value();
     Executor executor_config = mod->GetAttr<Executor>(tvm::attr::kExecutor).value();
     String interface_api = executor_config->GetAttr<String>("interface-api").value_or("packed");
     Integer workspace_byte_alignment =
         executor_config->GetAttr<Integer>("workspace-byte-alignment").value_or(16);
     use_unpacked_api_ = executor_config->GetAttr<Bool>("unpacked-api").value_or(Bool(false));
+    use_call_cpacked_ = Bool(interface_api == "c");
+
+    // Validate choice of use_unpacked_api_ and use_call_cpacked_
+    if (runtime_config->name == kTvmRuntimeCrt) {
+      CHECK(interface_api == "c" || bool(use_unpacked_api_) == false)

Review comment:
       Don't compare `bool`s to `true` or `false`, use them directly.  The result of such a comparison is another `bool`...

##########
File path: python/tvm/contrib/hexagon/hexagon.py
##########
@@ -248,3 +262,24 @@ def transform(func, mod, ctx):
 
 def ir_lower_vtcm_pass():
     return [(3, ir_lower_vtcm())]
+
+@register_func("tvm.contrib.hexagon.hexagon.aot_export")
+def aot_export(so_name, files, **kwargs):
+    tvm_dir = pathlib.Path(os.path.dirname(os.path.realpath(__file__))) / ".." / ".." / ".." / ".."
+    options = [
+        f"-I{tvm_dir / 'include'}",
+        f"-I{tvm_dir / '3rdparty' / 'dlpack' / 'include'}",
+        f"-I{tvm_dir / '3rdparty' / 'dmlc-core' / 'include'}",
+        f"-I{tvm_dir / 'src' / 'runtime' / 'hexagon' / 'android' / 'sim' / 'driver'}",

Review comment:
       You shouldn't be using that.  What do you need from there?  If pthreads, then you need to get that from the SDK.

##########
File path: include/tvm/runtime/metadata.h
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/metadata.h
+ * \brief Defines types which can be used in Metadata.
+ */
+#ifndef TVM_RUNTIME_METADATA_H_
+#define TVM_RUNTIME_METADATA_H_
+
+#include <inttypes.h>
+#include <tvm/runtime/c_runtime_api.h>
+#include <tvm/runtime/metadata_base.h>
+#include <tvm/support/span.h>
+
+#define TVM_METADATA_VERSION 1
+static const constexpr int64_t kMetadataVersion = TVM_METADATA_VERSION;
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct TVMMetadata {
+  int64_t version;
+  const struct TVMTensorInfo* inputs;
+  int64_t num_inputs;
+  const struct TVMTensorInfo* outputs;
+  int64_t num_outputs;
+  const char** devices;
+  int64_t num_devices;
+  const char* executor;
+  const char* mod_name;
+  const char* interface_api;
+  bool use_unpacked_api;
+};
+
+struct TVMTensorInfo {
+  const char* name;
+  const int64_t* shape;
+  int64_t num_shape;
+  DLDataType dtype;
+};
+#ifdef __cplusplus
+}  // extern "C"
+#include <tvm/runtime/object.h>
+namespace tvm {
+namespace runtime {
+namespace metadata {
+
+class Metadata;
+class TensorInfo;
+
+class MetadataNode : public MetadataBaseNode {
+ public:
+  MetadataNode(const struct ::TVMMetadata* data) : data_{data} {}
+  static constexpr const char* _type_key = "metadata.MetadataNode";
+  std::string get_name() override;
+  inline int64_t version() const { return int64_t(data_->version); }
+  inline int64_t num_inputs() const { return data_->num_inputs; }
+  ArrayAccessor<struct TVMTensorInfo, TensorInfo> inputs();
+  inline int64_t num_outputs() const { return data_->num_outputs; }
+  ArrayAccessor<struct TVMTensorInfo, TensorInfo> outputs();
+  inline int64_t num_devices() const { return data_->num_devices; }
+  ArrayAccessor<const char*, ::tvm::runtime::String> devices();
+  inline ::tvm::runtime::String executor() const { return ::tvm::runtime::String(data_->executor); }

Review comment:
       The `::` in `::tvm` and `::std` is unnecessary.

##########
File path: include/tvm/runtime/metadata_base.h
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/metadata_base.h
+ * \brief Defines types which can be used in Metadata.
+ */
+#ifndef TVM_RUNTIME_METADATA_BASE_H_
+#define TVM_RUNTIME_METADATA_BASE_H_
+
+#include <string>
+#include <tvm/ir/expr.h>
+#include <tvm/runtime/object.h>
+
+namespace tvm {
+namespace runtime {
+namespace metadata {
+
+class MetadataBaseNode : public ::tvm::runtime::Object {
+ public:
+  virtual std::string get_name() = 0;
+
+  static constexpr const char* _type_key = "metadata.MetadataBaseNode";
+  TVM_DECLARE_BASE_OBJECT_INFO(MetadataBaseNode, ::tvm::runtime::Object);
+};
+
+class MetadataBase : public ::tvm::runtime::ObjectRef {
+ public:
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(MetadataBase, ::tvm::runtime::ObjectRef, MetadataBaseNode);
+};
+
+template <typename C, class Ref>
+class ArrayAccessor;

Review comment:
       The classes `ArrayAccessor` and `ArrayIterator` don't seem to be used anywhere.

##########
File path: include/tvm/runtime/metadata_base.h
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/metadata_base.h
+ * \brief Defines types which can be used in Metadata.
+ */
+#ifndef TVM_RUNTIME_METADATA_BASE_H_
+#define TVM_RUNTIME_METADATA_BASE_H_
+
+#include <string>
+#include <tvm/ir/expr.h>
+#include <tvm/runtime/object.h>
+
+namespace tvm {
+namespace runtime {
+namespace metadata {
+
+class MetadataBaseNode : public ::tvm::runtime::Object {
+ public:
+  virtual std::string get_name() = 0;
+
+  static constexpr const char* _type_key = "metadata.MetadataBaseNode";
+  TVM_DECLARE_BASE_OBJECT_INFO(MetadataBaseNode, ::tvm::runtime::Object);
+};
+
+class MetadataBase : public ::tvm::runtime::ObjectRef {
+ public:
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(MetadataBase, ::tvm::runtime::ObjectRef, MetadataBaseNode);
+};
+
+template <typename C, class Ref>

Review comment:
       `class Ref` -> `typename Ref`




-- 
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] mehrdadh commented on a change in pull request #10311: [runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r819113181



##########
File path: tests/python/contrib/test_hexagon/test_launcher.py
##########
@@ -50,12 +51,12 @@ def test_add(android_serial_number, tvm_tracker_host, tvm_tracker_port):
     func.save(dso_binary_path)
 
     if not android_serial_number:
-        pytest.skip("Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
+        pytest.skip(msg="Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
 
     rpc_info = {
-      "rpc_tracker_host" : tvm_tracker_host,
-      "rpc_tracker_port" : tvm_tracker_port,
-      "rpc_server_port" : 7070,
+        "rpc_tracker_host": tvm_tracker_host,
+        "rpc_tracker_port": tvm_tracker_port,
+        "rpc_server_port": 7070,

Review comment:
       Yeah we still have that issue. I think we should try to fix it before merging your PR on adding the simulator, otherwise it would become flaky and people complain. Have you seen this issue with running on simulator?




-- 
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 #10311: [Draft][runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r815232003



##########
File path: include/tvm/runtime/metadata.h
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/metadata.h
+ * \brief Defines types which can be used in Metadata.
+ */
+#ifndef TVM_RUNTIME_METADATA_H_
+#define TVM_RUNTIME_METADATA_H_
+
+#include <inttypes.h>
+#include <tvm/runtime/c_runtime_api.h>
+#include <tvm/runtime/metadata_base.h>
+#include <tvm/support/span.h>
+
+#define TVM_METADATA_VERSION 1
+static const constexpr int64_t kMetadataVersion = TVM_METADATA_VERSION;
+#ifdef __cplusplus

Review comment:
       in #10283, I moved the `#ifdef` to exclude any c++ syntax and test_cpp_aot now proves that this file compiles under C. it is my intent that this file can be compiled without the need for C++.

##########
File path: include/tvm/runtime/metadata.h
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/metadata.h
+ * \brief Defines types which can be used in Metadata.
+ */
+#ifndef TVM_RUNTIME_METADATA_H_
+#define TVM_RUNTIME_METADATA_H_
+
+#include <inttypes.h>
+#include <tvm/runtime/c_runtime_api.h>
+#include <tvm/runtime/metadata_base.h>
+#include <tvm/support/span.h>
+
+#define TVM_METADATA_VERSION 1
+static const constexpr int64_t kMetadataVersion = TVM_METADATA_VERSION;
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct TVMMetadata {
+  int64_t version;
+  const struct TVMTensorInfo* inputs;
+  int64_t num_inputs;
+  const struct TVMTensorInfo* outputs;
+  int64_t num_outputs;
+  const char** devices;
+  int64_t num_devices;
+  const char* executor;
+  const char* mod_name;
+  const char* interface_api;
+  bool use_unpacked_api;
+};
+
+struct TVMTensorInfo {
+  const char* name;
+  const int64_t* shape;
+  int64_t num_shape;
+  DLDataType dtype;
+};
+#ifdef __cplusplus

Review comment:
       let's discuss above

##########
File path: include/tvm/runtime/metadata.h
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/metadata.h
+ * \brief Defines types which can be used in Metadata.
+ */
+#ifndef TVM_RUNTIME_METADATA_H_
+#define TVM_RUNTIME_METADATA_H_
+
+#include <inttypes.h>
+#include <tvm/runtime/c_runtime_api.h>
+#include <tvm/runtime/metadata_base.h>
+#include <tvm/support/span.h>
+
+#define TVM_METADATA_VERSION 1
+static const constexpr int64_t kMetadataVersion = TVM_METADATA_VERSION;
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct TVMMetadata {
+  int64_t version;
+  const struct TVMTensorInfo* inputs;
+  int64_t num_inputs;
+  const struct TVMTensorInfo* outputs;
+  int64_t num_outputs;
+  const char** devices;
+  int64_t num_devices;
+  const char* executor;
+  const char* mod_name;
+  const char* interface_api;
+  bool use_unpacked_api;
+};
+
+struct TVMTensorInfo {
+  const char* name;
+  const int64_t* shape;
+  int64_t num_shape;
+  DLDataType dtype;
+};
+#ifdef __cplusplus
+}  // extern "C"
+#include <tvm/runtime/object.h>
+namespace tvm {
+namespace runtime {
+namespace metadata {
+
+class Metadata;
+class TensorInfo;
+
+class MetadataNode : public MetadataBaseNode {
+ public:
+  MetadataNode(const struct ::TVMMetadata* data) : data_{data} {}
+  static constexpr const char* _type_key = "metadata.MetadataNode";
+  std::string get_name() override;
+  inline int64_t version() const { return int64_t(data_->version); }
+  inline int64_t num_inputs() const { return data_->num_inputs; }
+  ArrayAccessor<struct TVMTensorInfo, TensorInfo> inputs();
+  inline int64_t num_outputs() const { return data_->num_outputs; }
+  ArrayAccessor<struct TVMTensorInfo, TensorInfo> outputs();
+  inline int64_t num_devices() const { return data_->num_devices; }
+  ArrayAccessor<const char*, ::tvm::runtime::String> devices();
+  inline ::tvm::runtime::String executor() const { return ::tvm::runtime::String(data_->executor); }

Review comment:
       i'll address this in #10283

##########
File path: include/tvm/runtime/metadata_base.h
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/metadata_base.h
+ * \brief Defines types which can be used in Metadata.
+ */
+#ifndef TVM_RUNTIME_METADATA_BASE_H_
+#define TVM_RUNTIME_METADATA_BASE_H_
+
+#include <string>
+#include <tvm/ir/expr.h>
+#include <tvm/runtime/object.h>
+
+namespace tvm {
+namespace runtime {
+namespace metadata {
+
+class MetadataBaseNode : public ::tvm::runtime::Object {
+ public:
+  virtual std::string get_name() = 0;
+
+  static constexpr const char* _type_key = "metadata.MetadataBaseNode";
+  TVM_DECLARE_BASE_OBJECT_INFO(MetadataBaseNode, ::tvm::runtime::Object);
+};
+
+class MetadataBase : public ::tvm::runtime::ObjectRef {
+ public:
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(MetadataBase, ::tvm::runtime::ObjectRef, MetadataBaseNode);
+};
+
+template <typename C, class Ref>

Review comment:
       i'll address this in #10283




-- 
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] kparzysz-quic commented on a change in pull request #10311: [runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r819115839



##########
File path: tests/python/contrib/test_hexagon/test_launcher.py
##########
@@ -50,12 +51,12 @@ def test_add(android_serial_number, tvm_tracker_host, tvm_tracker_port):
     func.save(dso_binary_path)
 
     if not android_serial_number:
-        pytest.skip("Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
+        pytest.skip(msg="Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
 
     rpc_info = {
-      "rpc_tracker_host" : tvm_tracker_host,
-      "rpc_tracker_port" : tvm_tracker_port,
-      "rpc_server_port" : 7070,
+        "rpc_tracker_host": tvm_tracker_host,
+        "rpc_tracker_port": tvm_tracker_port,
+        "rpc_server_port": 7070,

Review comment:
       I haven't seen it outside of docker.  I don't know how to fix it, other than adding a sleep that will wait long enough for the socket to become available again...  Using different port numbers is guaranteed to work, but it requires paying attention when writing tests---it would be easy to make a mistake.
   
   Edit: I don't know why I didn't see this issue when I ran it directly (i.e. not in docker).    ( ゚o゚)




-- 
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 #10311: [Draft][runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r815232248



##########
File path: include/tvm/runtime/metadata_base.h
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/metadata_base.h
+ * \brief Defines types which can be used in Metadata.
+ */
+#ifndef TVM_RUNTIME_METADATA_BASE_H_
+#define TVM_RUNTIME_METADATA_BASE_H_
+
+#include <string>
+#include <tvm/ir/expr.h>
+#include <tvm/runtime/object.h>
+
+namespace tvm {
+namespace runtime {
+namespace metadata {
+
+class MetadataBaseNode : public ::tvm::runtime::Object {
+ public:
+  virtual std::string get_name() = 0;
+
+  static constexpr const char* _type_key = "metadata.MetadataBaseNode";
+  TVM_DECLARE_BASE_OBJECT_INFO(MetadataBaseNode, ::tvm::runtime::Object);
+};
+
+class MetadataBase : public ::tvm::runtime::ObjectRef {
+ public:
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(MetadataBase, ::tvm::runtime::ObjectRef, MetadataBaseNode);
+};
+
+template <typename C, class Ref>
+class ArrayAccessor;

Review comment:
       they should be used in `src/runtime/metadata.cc`.

##########
File path: src/relay/backend/aot_executor_codegen.cc
##########
@@ -819,11 +854,29 @@ class AOTExecutorCodegen : public MixedModeVisitor {
     ICHECK(target_host_.defined()) << "require a target_host to be given for AOT codegen";
     VLOG(1) << "target host: " << target_host_->ToDebugString();
 
+    Runtime runtime_config = mod->GetAttr<Runtime>(tvm::attr::kRuntime).value();
     Executor executor_config = mod->GetAttr<Executor>(tvm::attr::kExecutor).value();
     String interface_api = executor_config->GetAttr<String>("interface-api").value_or("packed");
     Integer workspace_byte_alignment =
         executor_config->GetAttr<Integer>("workspace-byte-alignment").value_or(16);
     use_unpacked_api_ = executor_config->GetAttr<Bool>("unpacked-api").value_or(Bool(false));
+    use_call_cpacked_ = Bool(interface_api == "c");
+
+    // Validate choice of use_unpacked_api_ and use_call_cpacked_
+    if (runtime_config->name == kTvmRuntimeCrt) {
+      CHECK(interface_api == "c" || bool(use_unpacked_api_) == false)

Review comment:
       i'll address this in #10283




-- 
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] kparzysz-quic commented on a change in pull request #10311: [runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r819115839



##########
File path: tests/python/contrib/test_hexagon/test_launcher.py
##########
@@ -50,12 +51,12 @@ def test_add(android_serial_number, tvm_tracker_host, tvm_tracker_port):
     func.save(dso_binary_path)
 
     if not android_serial_number:
-        pytest.skip("Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
+        pytest.skip(msg="Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
 
     rpc_info = {
-      "rpc_tracker_host" : tvm_tracker_host,
-      "rpc_tracker_port" : tvm_tracker_port,
-      "rpc_server_port" : 7070,
+        "rpc_tracker_host": tvm_tracker_host,
+        "rpc_tracker_port": tvm_tracker_port,
+        "rpc_server_port": 7070,

Review comment:
       I haven't seen it outside of docker.  I don't know how to fix it, other than adding a sleep that will wait long enough for the socket to become available again...  Using different port numbers is guaranteed to work, but it requires paying attention when writing tests---it would be easy to make a mistake.




-- 
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] mehrdadh commented on a change in pull request #10311: [runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r819018758



##########
File path: python/tvm/contrib/hexagon/hexagon.py
##########
@@ -248,3 +265,44 @@ def transform(func, mod, ctx):
 
 def ir_lower_vtcm_pass():
     return [(3, ir_lower_vtcm())]
+
+
+def create_aot_shared(so_name: Union[str, pathlib.Path], files, hexagon_arch: str, options=None):
+    """Export Hexagon AOT module."""
+    if not os.access(str(HEXAGON_CLANG_PLUS), os.X_OK):
+        raise Exception(
+            'The Clang++ "' + str(HEXAGON_CLANG_PLUS) + '" does not exist or is not executable.'
+        )
+    if not HEXAGON_TOOLCHAIN:
+        raise Exception(
+            " The environment variable HEXAGON_TOOLCHAIN is unset. Please export "
+            + "HEXAGON_TOOLCHAIN in your environment."
+        )
+    if not HEXAGON_SDK_PATH:
+        raise Exception(
+            " The environment variable HEXAGON_SDK_PATH is unset. Please export "
+            + "HEXAGON_SDK_PATH in your environment."
+        )
+
+    tvm_dir = pathlib.Path(os.path.dirname(os.path.realpath(__file__))) / ".." / ".." / ".." / ".."
+    compute_arch = f"compute{hexagon_arch}"
+    compile_options = [
+        f"-I{tvm_dir / 'include'}",
+        f"-I{tvm_dir / '3rdparty' / 'dlpack' / 'include'}",
+        f"-I{tvm_dir / '3rdparty' / 'dmlc-core' / 'include'}",
+        f"-I{pathlib.Path(HEXAGON_SDK_PATH) / 'rtos' / 'qurt' / compute_arch / 'include'/ 'posix'}",
+        f"-I{pathlib.Path(HEXAGON_SDK_PATH) / 'rtos' / 'qurt' / compute_arch / 'include' / 'qurt'}",
+        f"-DDMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>",
+        f"-D_MACH_I32=int",
+    ]
+
+    # For debugging
+    for path in HEXAGON_SDK_INCLUDE_DIRS:
+        compile_options.append(f"-I{str(path)}")
+
+    cross_compile = cc.cross_compiler(
+        compile_func=tvm.get_global_func("tvm.contrib.hexagon.hexagon.hexagon_clang_plus")()

Review comment:
       no reason for it. I followed the approach for [hexagon_link](https://github.com/apache/tvm/blob/a01b3890ec53ce8e288917c856af3cecc10a2aac/python/tvm/contrib/hexagon/hexagon.py#L65). Maybe there was a reason there? but I don't think I need it for clang_plus. I will address it in a follow up if nothing major was mentioned in the reviews.




-- 
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] mehrdadh commented on a change in pull request #10311: [Draft][runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r816057421



##########
File path: python/tvm/contrib/hexagon/hexagon.py
##########
@@ -248,3 +262,24 @@ def transform(func, mod, ctx):
 
 def ir_lower_vtcm_pass():
     return [(3, ir_lower_vtcm())]
+
+@register_func("tvm.contrib.hexagon.hexagon.aot_export")
+def aot_export(so_name, files, **kwargs):
+    tvm_dir = pathlib.Path(os.path.dirname(os.path.realpath(__file__))) / ".." / ".." / ".." / ".."
+    options = [
+        f"-I{tvm_dir / 'include'}",
+        f"-I{tvm_dir / '3rdparty' / 'dlpack' / 'include'}",
+        f"-I{tvm_dir / '3rdparty' / 'dmlc-core' / 'include'}",
+        f"-I{tvm_dir / 'src' / 'runtime' / 'hexagon' / 'android' / 'sim' / 'driver'}",

Review comment:
       that's right. Will fix this before changing PR to ready for review.




-- 
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 #10311: [Draft][runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r814167425



##########
File path: src/runtime/hexagon/hexagon/hexagon_device_api_v2.cc
##########
@@ -82,7 +82,11 @@ void* HexagonDeviceAPIv2::AllocDataSpace(Device dev, size_t nbytes, size_t align
 }
 
 void HexagonDeviceAPIv2::FreeDataSpace(Device dev, void* ptr) {
-  CHECK(TVMDeviceExtType(dev.device_type) == kDLHexagon);
+  bool device = false;
+  if ((TVMDeviceExtType(dev.device_type) == kDLHexagon) || (DLDeviceType(dev.device_type) == kDLCPU)) {

Review comment:
       on AOT we currently require device_type == kDLCPU. this hack seems consistent with the previous approach to patch the CPU device with Hexagon. we could add a comment here to explain. 
   
   imo we should take up the issue of what device type to actually use here when we address the compilation path for Hexagon code.




-- 
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 #10311: [runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r819001845



##########
File path: src/runtime/hexagon/hexagon/hexagon_device_api_v2.cc
##########
@@ -84,7 +84,12 @@ void* HexagonDeviceAPIv2::AllocDataSpace(Device dev, size_t nbytes, size_t align
 }
 
 void HexagonDeviceAPIv2::FreeDataSpace(Device dev, void* ptr) {
-  CHECK(TVMDeviceExtType(dev.device_type) == kDLHexagon);
+  bool device = false;
+  if ((TVMDeviceExtType(dev.device_type) == kDLHexagon) ||
+      (DLDeviceType(dev.device_type) == kDLCPU)) {
+    device = true;
+  }
+  CHECK(device) << "dev.device_type: " << dev.device_type;

Review comment:
       ```suggestion
     bool is_valid_device = 
         (TVMDeviceExtType(dev.device_type) == kDLHexagon) ||
         (DLDeviceType(dev.device_type) == kDLCPU);
     CHECK(is_valid_device) << "dev.device_type: " << dev.device_type;
   ```

##########
File path: python/tvm/contrib/hexagon/hexagon.py
##########
@@ -248,3 +265,44 @@ def transform(func, mod, ctx):
 
 def ir_lower_vtcm_pass():
     return [(3, ir_lower_vtcm())]
+
+
+def create_aot_shared(so_name: Union[str, pathlib.Path], files, hexagon_arch: str, options=None):
+    """Export Hexagon AOT module."""
+    if not os.access(str(HEXAGON_CLANG_PLUS), os.X_OK):
+        raise Exception(
+            'The Clang++ "' + str(HEXAGON_CLANG_PLUS) + '" does not exist or is not executable.'
+        )
+    if not HEXAGON_TOOLCHAIN:
+        raise Exception(
+            " The environment variable HEXAGON_TOOLCHAIN is unset. Please export "
+            + "HEXAGON_TOOLCHAIN in your environment."
+        )
+    if not HEXAGON_SDK_PATH:
+        raise Exception(
+            " The environment variable HEXAGON_SDK_PATH is unset. Please export "
+            + "HEXAGON_SDK_PATH in your environment."
+        )
+
+    tvm_dir = pathlib.Path(os.path.dirname(os.path.realpath(__file__))) / ".." / ".." / ".." / ".."
+    compute_arch = f"compute{hexagon_arch}"
+    compile_options = [
+        f"-I{tvm_dir / 'include'}",
+        f"-I{tvm_dir / '3rdparty' / 'dlpack' / 'include'}",
+        f"-I{tvm_dir / '3rdparty' / 'dmlc-core' / 'include'}",
+        f"-I{pathlib.Path(HEXAGON_SDK_PATH) / 'rtos' / 'qurt' / compute_arch / 'include'/ 'posix'}",
+        f"-I{pathlib.Path(HEXAGON_SDK_PATH) / 'rtos' / 'qurt' / compute_arch / 'include' / 'qurt'}",
+        f"-DDMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>",
+        f"-D_MACH_I32=int",
+    ]
+
+    # For debugging
+    for path in HEXAGON_SDK_INCLUDE_DIRS:
+        compile_options.append(f"-I{str(path)}")
+
+    cross_compile = cc.cross_compiler(
+        compile_func=tvm.get_global_func("tvm.contrib.hexagon.hexagon.hexagon_clang_plus")()

Review comment:
       just curious, why do we register a PackedFunc to return a constant to python?




-- 
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 #10311: [runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
masahi merged pull request #10311:
URL: https://github.com/apache/tvm/pull/10311


   


-- 
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] mehrdadh commented on a change in pull request #10311: [runtime][Hexagon] AOTExecutor implementation for C Codegen

Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #10311:
URL: https://github.com/apache/tvm/pull/10311#discussion_r819110837



##########
File path: tests/python/contrib/test_hexagon/test_launcher.py
##########
@@ -217,24 +223,23 @@ def test_graph_executor(android_serial_number, tvm_tracker_host, tvm_tracker_por
         lowered.get_lib().save(dso_binary_path)
 
     if not android_serial_number:
-        pytest.skip("Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
+        pytest.skip(msg="Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")
 
     rpc_info = {
-      "rpc_tracker_host" : tvm_tracker_host,
-      "rpc_tracker_port" : tvm_tracker_port,
-      "rpc_server_port" : 7070,
+        "rpc_tracker_host": tvm_tracker_host,
+        "rpc_tracker_port": tvm_tracker_port,
+        "rpc_server_port": 7070,
     }
     launcher = HexagonLauncher(serial_number=android_serial_number, rpc_info=rpc_info)
     launcher.upload(dso_binary_path, dso_binary)
     launcher.start_server()
 
     with launcher.start_session() as sess:
         graph_mod = launcher.get_graph_executor(lowered.get_graph_json(), dso_binary, sess)
-        weight_in = np.random.rand(5, 5, 3, 8).astype(dtype=dtype)
-        data_in = np.random.rand(1, 64, 64, 3).astype(dtype=dtype)
-        graph_mod.set_input(weight=weight_in)
-        graph_mod.run(data=data_in)
+        graph_mod.set_input(**params)
+        graph_mod.run(**inputs)
         hexagon_output = graph_mod.get_output(0).numpy()
+        launcher.stop_server()

Review comment:
       makes sense. I will fix this in a follow up PR if CI stays green. I have another PR which fixes few things to be able to run the tests on hardware nightly




-- 
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