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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to