[GitHub] [tvm] manupa-arm commented on a change in pull request #10282: [runtime] Add Metadata classes for AOTExecutor

[GitBox <gi...@apache.org>]

manupa-arm commented on a change in pull request #10282:
URL: https://github.com/apache/tvm/pull/10282#discussion_r810954339



##########
File path: include/tvm/runtime/metadata.h
##########
@@ -0,0 +1,125 @@
+/*
+ * 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>
+#ifdef __cplusplus
+#include <memory>
+#include <string>
+#include <vector>
+#endif
+#include <tvm/runtime/c_runtime_api.h>
+#ifdef __cplusplus
+#include <tvm/runtime/metadata_base.h>
+#endif
+#include <tvm/support/span.h>
+
+// Version number recorded in emitted artifacts for runtime checking.
+#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;

Review comment:
       Should we have docs for definitions in public facing headers ?

##########
File path: include/tvm/support/span.h
##########
@@ -0,0 +1,98 @@
+/*
+ * 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/support/span.h
+ * \brief Reimplementation of part of C++-20 style span.
+ */
+#ifndef TVM_SUPPORT_SPAN_H_
+#define TVM_SUPPORT_SPAN_H_
+
+#include <cstddef>
+#include <iterator>
+#include <type_traits>
+#include <vector>
+
+namespace tvm {
+namespace support {
+
+template <class T, class W>
+class Span {

Review comment:
       We need to disambiguate this from include/tvm/ir/span.h .
   Can you provide more info ?

##########
File path: include/tvm/runtime/metadata.h
##########
@@ -0,0 +1,125 @@
+/*
+ * 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>
+#ifdef __cplusplus
+#include <memory>
+#include <string>
+#include <vector>
+#endif
+#include <tvm/runtime/c_runtime_api.h>
+#ifdef __cplusplus
+#include <tvm/runtime/metadata_base.h>
+#endif
+#include <tvm/support/span.h>
+
+// Version number recorded in emitted artifacts for runtime checking.
+#define TVM_METADATA_VERSION 1
+static const constexpr int64_t kMetadataVersion = TVM_METADATA_VERSION;

Review comment:
       Shall we specialize this to be kRuntimeMetadataVersion ? -- mainly because namespaces are not there in C.

##########
File path: src/target/metadata.h
##########
@@ -0,0 +1,159 @@
+/*
+ * 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/target/metadata.h
+ * \brief Extends Metadata for use in the compiler.
+ */
+#ifndef TVM_TARGET_METADATA_H_
+#define TVM_TARGET_METADATA_H_
+
+#include <tvm/runtime/metadata.h>
+
+#include <memory>
+#include <string>
+#include <vector>
+
+namespace tvm {
+namespace target {
+namespace metadata {
+
+class VisitableMetadataNode : public ::tvm::runtime::metadata::MetadataNode {

Review comment:
       I think we need some docs explaining the seperation of VisitableMetadataNode vs MetadataNode. It is not entirely clear to me. Is it something to do with being able serialize/deserialize ?

##########
File path: src/target/metadata.h
##########
@@ -0,0 +1,159 @@
+/*
+ * 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/target/metadata.h
+ * \brief Extends Metadata for use in the compiler.
+ */
+#ifndef TVM_TARGET_METADATA_H_
+#define TVM_TARGET_METADATA_H_
+
+#include <tvm/runtime/metadata.h>
+
+#include <memory>
+#include <string>
+#include <vector>
+
+namespace tvm {
+namespace target {
+namespace metadata {
+
+class VisitableMetadataNode : public ::tvm::runtime::metadata::MetadataNode {
+ public:
+  explicit VisitableMetadataNode(const struct ::TVMMetadata* data) : MetadataNode{data} {}
+  VisitableMetadataNode() : MetadataNode{nullptr} {}
+
+  void VisitAttrs(AttrVisitor* v) {
+    int64_t version_cpp{version()};
+    v->Visit("version", &version_cpp);
+    auto inputs_array = Array<ObjectRef>();
+    auto inputs_accessor = inputs();
+    inputs_array.reserve(num_inputs());
+    for (int64_t i = 0; i < num_inputs(); ++i) {
+      inputs_array.push_back(::tvm::runtime::metadata::TensorInfo{inputs_accessor[i]});
+    }
+    ::tvm::runtime::metadata::MetadataArray inputs_metadata_array{
+        inputs_array, ::tvm::runtime::metadata::MetadataTypeIndex::kMetadata, "TVMTensorInfo"};
+    v->Visit("inputs", &inputs_metadata_array);
+    int64_t num_inputs_cpp = num_inputs();
+    v->Visit("num_inputs", &num_inputs_cpp);
+    auto outputs_array = Array<ObjectRef>();
+    auto outputs_accessor = outputs();
+    outputs_array.reserve(num_outputs());
+    for (int64_t i = 0; i < num_outputs(); ++i) {
+      outputs_array.push_back(::tvm::runtime::metadata::TensorInfo{outputs_accessor[i]});
+    }
+    ::tvm::runtime::metadata::MetadataArray outputs_metadata_array{
+        outputs_array, ::tvm::runtime::metadata::MetadataTypeIndex::kMetadata, "TVMTensorInfo"};
+    v->Visit("outputs", &outputs_metadata_array);
+    int64_t num_outputs_cpp = num_outputs();
+    v->Visit("num_outputs", &num_outputs_cpp);
+    ::std::string mod_name_cpp{data()->mod_name};
+    v->Visit("mod_name", &mod_name_cpp);
+  }
+};
+
+class InMemoryMetadataNode : public ::tvm::target::metadata::VisitableMetadataNode {

Review comment:
       Can you provide more explanation for the variants of InMemory* ? Also the reasons for needing a seperate object for that as opposed to having a constructor for MetadataNode

##########
File path: include/tvm/runtime/metadata_base.h
##########
@@ -0,0 +1,176 @@
+/*
+ * 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 <tvm/ir/expr.h>
+#include <tvm/runtime/object.h>
+
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+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;
+
+template <typename C, class Ref>
+class ArrayIterator {

Review comment:
       Im not sure whether these belong here... seems like utilities to accessing Array(-ish structures)s. Am I right ? 
   
   Maybe in include/tvm/runtime/container/array.h ?
   
   Moreover, I think we need docs to understand their purpose I dont fully follow what these utilities are meant to do.
   

##########
File path: include/tvm/runtime/metadata_base.h
##########
@@ -0,0 +1,176 @@
+/*
+ * 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 <tvm/ir/expr.h>
+#include <tvm/runtime/object.h>
+
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+namespace tvm {
+namespace runtime {
+namespace metadata {
+
+class MetadataBaseNode : public ::tvm::runtime::Object {

Review comment:
       I think docs for this base class would be helpful to understand what needs to go here as opposed to inherited classes




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