[runtime] AOTExecutor implementation and c target code-generator #10283

areusch · 2022-02-17T03:25:23Z

This PR adds the AOTExecutor implementation and the metadata code-generator for use with the c target. This PR allows users to run models which use only the CPU via the AOTExecutor using the C++ runtime.

cc @Mousius @manupa-arm @kparzysz-quic @masahi @mehrdadh

tests/python/relay/aot/test_cpp_aot.py

kparzysz-quic

Looks ok to me. Please change the make_unique statements though, but it can go into a subsequent PR.

kparzysz-quic · 2022-02-28T20:01:17Z

src/runtime/aot_executor/aot_executor.cc

+
+  const int num_args = args_.size();
+  ::std::unique_ptr<TVMValue> call_values{new TVMValue[num_args]};
+  ::std::unique_ptr<int> call_type_codes{new int[num_args]};


These two lines should be

auto call_values = std::make_unique<TVMValue[]>(num_args); auto call_type_codes = std::make_unique<int[]>(num_args);

fixed, thanks

kparzysz-quic · 2022-02-28T21:11:15Z

src/relay/backend/aot_executor_codegen.cc

+      return_ttypes_.reserve(ttypes.size());
+      for (auto ttype : ttypes) {
+        return_ttypes_.push_back(ttype);
+      }


All of this is equivalent to return_ttypes_ = FlattenTupleType(e->checked_type());.

cc @areusch

masahi

Found some left-over log while running test_conv2d

masahi · 2022-03-01T10:43:22Z

src/target/source/source_module.cc

+      auto metadata = Downcast<runtime::metadata::MetadataBase>(*value);
+      const runtime::metadata::MetadataArrayNode* arr =
+          value->as<runtime::metadata::MetadataArrayNode>();
+      std::cout << "Is array? " << arr << std::endl;


masahi · 2022-03-01T10:43:38Z

src/target/source/source_module.cc

+  void Visit(const char* key, ObjectRef* value) final {
+    const runtime::metadata::MetadataArrayNode* arr =
+        value->as<runtime::metadata::MetadataArrayNode>();
+    std::cout << "Is array? " << arr << std::endl;


masahi · 2022-03-01T10:44:15Z

src/target/source/source_module.cc

+    for (unsigned int i = 0; i < array->array.size(); ++i) {  // ObjectRef o : *(array->array)) {
+      ObjectRef o = array->array[i];
+      std::cout << "visiting array element " << i << ": " << o->type_index() << " ("
+                << o.operator->() << ")" << std::endl;


masahi · 2022-03-01T10:44:35Z

src/target/source/source_module.cc

+        for (unsigned int i = 0; i < arr->array.size(); i++) {
+          ObjectRef o = arr->array[i];
+          std::cout << "queue-visiting array element " << i << ": " << o->type_index() << " ("
+                    << o.operator->() << ")" << std::endl;


masahi · 2022-03-01T10:45:00Z

src/target/source/source_module.cc

+      auto struct_name = std::get<0>(item);
+      auto obj = std::get<1>(item);
+      auto arr = obj.as<runtime::metadata::MetadataArrayNode>();
+      std::cout << "codegen: " << struct_name;


masahi · 2022-03-01T10:49:15Z

src/target/metadata_module.cc

+    pools.push_back(
+        runtime::metadata::TensorInfo(make_object<target::metadata::InMemoryTensorInfoNode>(
+            var->name_hint,
+            std::vector<int64_t>{metadata->pool_inputs.value()[var]->allocated_size},


Check that metadata->pool_inputs is non-null (it's Optional<Map<tir::Var, tir::usmp::AllocatedPoolInfo>>)

Is it done?

it is done earlier in the function, but it was in the wrong spot and i've reorganized the checks.

masahi · 2022-03-01T11:05:17Z

python/tvm/runtime/executor/aot_executor.py

+        lib: tvm.runtime.Module = tvm.runtime.load_module("compiled_lib.so")
+        # Call the library factory function for default and create
+        # a new runtime.Module, wrap with graph module.
+        gmod = graph_executor.GraphModule(lib["default"](dev))


Need update?

fixed, thanks!

masahi · 2022-03-01T11:10:47Z

python/tvm/runtime/executor/__init__.py

+ - GraphExecutor, in python/tvm/contrib/graph_executor.py
+ - VM Executor, in python/tvm/runtime/vm.py
+
+TODO(areusch): Consolidate these into this module.


I just realized that, we have two notions of executor. One is the runtime one above, the other is

tvm/python/tvm/relay/build_module.py

Line 647 in 55849e6

def create_executor(kind="debug", mod=None, device=None, target="llvm", params=None):

which is used a lot in the test cases.

Do we intend to support create_executor(kind="aot", ...), given that we can now run things via the cpp runtime?

yeah that is a good point. added support here.

masahi · 2022-03-01T11:21:03Z

include/tvm/runtime/metadata.h

@@ -75,6 +75,13 @@ struct TVMMetadata {
  const struct TVMTensorInfo* outputs;
  /*! \brief Number of elements in `outputs` array. */
  int64_t num_outputs;
+  /*! \brief Memory Pools needed by the AOT run_model function.


You are not using run_model anymore, are you? In your previous branch indeed I'm seeing tvmgen_default_run_model generated, but after rebase it is replaced with tvmgen_default___tvm_main__.

That is right!.

We had run (entry_point), run_model and tvm_main.
The entry_point is supposed to call run_model, however, run_model is identical to tvm_main -- therefore, it was removed due to no need of maintaining two symbols for relay and tir versions of main.

So I think it needs to be tvm_main now.

changed to just "AOT main" since the function should probably eventually be renamed based on mod_name.

manupak

Neat! Looking good.

Few nits and questions (for my understanding).

manupak · 2022-03-01T18:12:02Z

python/tvm/runtime/executor/aot_executor.py

+import numpy as np
+
+
+class AotModule(object):


(stylistic) : AotModule --> AOTModule or AoTModule ?

i kind of find it easier to read the acronyms if we use CapWords, this also follows https://www.python.org/dev/peps/pep-0008/#class-names which is linked from numpydoc

manupak · 2022-03-01T18:13:36Z

src/relay/backend/aot_executor_codegen.cc

@@ -317,6 +327,16 @@ class AOTExecutorCodegen : public MixedModeVisitor {
    }
  }

+  void PushArgs(const Expr& expr, std::vector<tir::Var> sids, Array<PrimExpr>* args) {


nit : any reason mixed pass-by-value, pass-by-(const)-references usage and pointers here ?

oops, sorry kinda rough. changed to pass-by-const-ref and pointer per c++ style guide.

manupak · 2022-03-01T18:15:13Z

src/relay/backend/aot_executor_codegen.cc

+
+    // Validate choice of use_unpacked_api_ and use_call_cpacked_
+    if (runtime_config->name == kTvmRuntimeCrt) {
+      CHECK(interface_api == "packed" || static_cast<bool>(use_unpacked_api_) == true)


Are both of these CHECKs tested ?

the CRT CHECK is thoroughly tested by existing code (ran into a lot of those tests in making CI happy). added tests for the C++ one.

manupak · 2022-03-01T18:18:03Z

src/runtime/aot_executor/aot_executor_factory.cc

+
+/*!
+ * \file aot_executor_factory.cc
+ * \brief Graph executor factory implementations


Need update

manupak · 2022-03-01T18:19:27Z

src/runtime/aot_executor/aot_executor_factory.cc

+namespace tvm {
+namespace runtime {
+
+AotExecutorFactory::AotExecutorFactory(


Just for my knowledge, I never understood why these are called "factories". What maybe the reason ? :)

it's because they are container objects which hold the constructor arguments to an executor. They also handle serializing those arguments during export_library and reconstructing them during load_module. they follow the factory pattern (e.g. a no-arg or less-args functor that constructs a given datatype), so that's why the name.

manupak · 2022-03-01T18:23:34Z

tests/python/relay/aot/test_crt_aot.py

-    with pytest.raises(tvm.TVMError, match="Packed interface required for packed operators"):
+    with pytest.raises(
+        tvm.TVMError,
+        match=re.escape(


From the above comment, I'd expect two of these tests here.

i think the check fails when both of these conditions are false, so we just need one to assert we get the error message. the remaining single-true cases are tested in various other tests, so i think we should be ok. but lmk if you have a more specific concern.

masahi

More minor comments, finished the whole thing.

At some point we should review our meta data situation... There are so many classes / files and they look quite messy.

masahi · 2022-03-01T19:42:25Z

src/relay/backend/aot_executor_codegen.cc

+          << ") or unpacked-api == true (got: " << use_unpacked_api_
+          << ") when targeting c runtime";
+    } else if (runtime_config->name == kTvmRuntimeCpp) {
+      CHECK(static_cast<bool>(use_unpacked_api_) == false &&


Should use ICHECK, also at L918

masahi · 2022-03-01T19:46:34Z

src/runtime/aot_executor/aot_executor.h

+   *  processor.
+   * \param devs The device of the host and devices where graph nodes will be
+   *  executed on.
+   * \param lookup_linked_param_func If given, a PackedFunc invoked to lookup linked parameters


lookup_linked_param_func not given in the param list

masahi · 2022-03-01T19:47:40Z

src/runtime/aot_executor/aot_executor.h

+   * \brief Initialize the AOT executor with metadata, runtime::Module, and device.
+   * \param module The module containing the compiled functions for the host
+   *  processor.
+   * \param devs The device of the host and devices where graph nodes will be


There is no "graph".

And why only the first element of devs is ever used in aot_executor.cc?

reworded the comment, added ICHECK, and tested.

masahi · 2022-03-01T19:53:45Z

src/runtime/metadata.cc

+    //   "
+    //   << "(when passed from runtime)";
+    metadata_ = metadata;
+    //    code_ = code;


masahi · 2022-03-01T19:54:23Z

src/runtime/metadata.cc

+          CHECK_EQ(ret_type_code, kTVMOpaqueHandle)
+              << "Expected kOpaqueHandle returned; got " << ret_type_code;
+          CHECK(ret_value.v_handle != nullptr) << "get_c_metadata returned nullptr";
+


ICHECK or ICHECK_EQ in this function

masahi · 2022-03-01T19:57:33Z

src/target/source/codegen_source_base.h

+ *        module.
+ * \param target_module The main TIR-lowered internal runtime module
+ * \param modules All the external modules that needs to be imported inside the metadata module(s).
+ * \param target The target that all the modules are compiled for


metadata not documented

masahi · 2022-03-01T20:07:41Z

src/target/source/source_module.cc

+      address_.push_back(i_str.str());
+      Visit(nullptr, &metadata);
+      address_.pop_back();
+      //      ReflectionVTable::Global()->VisitAttrs(metadata.operator->(), this);


masahi · 2022-03-01T20:08:10Z

src/target/source/source_module.cc

+
+  auto mod = MetadataModuleCreate(metadata);
+  std::vector<String> func_names{"get_c_metadata"};
+  // definer.GetOutput() +


masahi · 2022-03-01T20:11:42Z

src/target/source/source_module.cc

+  lookup_func << "};" << std::endl;
+
+  auto mod = MetadataModuleCreate(metadata);
+  std::vector<String> func_names{"get_c_metadata"};


Rather than hard code get_c_metadata everywhere, we should introduce a new symbol at

tvm/include/tvm/runtime/module.h

Line 219 in d8f639a

namespace symbol {

(similar to tvm_module_main)

masahi · 2022-03-01T20:13:07Z

src/tir/transforms/legalize_packed_calls.cc

+                               {sid_array, 0, tir::builtin::kArrDeviceType, kDLCPU})));
+            new_stmts.push_back(tir::Evaluate(
+                tvm::tir::Call(DataType::Handle(), tvm::tir::builtin::tvm_struct_set(),
+                               {sid_array, 0, tir::builtin::kArrDeviceId, 0})));


What are these changes for?

these changes fixed the flaky stack bug where device allocations were using bad threadlocalstorage for workspace. the fix was to explicitly initialize the device_id and device_type fields of the created DLTensor. there are other fields we need to initialize as well, but which we don't use in the c-codgen generated code, at least not extensively.

masahi · 2022-03-01T20:22:01Z

src/runtime/metadata.cc

+      return PackedFunc([this, sptr_to_self](TVMArgs args, TVMRetValue* rv) {
+        if (!metadata_.defined()) {
+          TVMFunctionHandle f_handle;
+          int32_t ret_code = TVMBackendGetFuncFromEnv(this, "get_c_metadata", &f_handle);


See https://github.com/apache/tvm/pull/10283/files#r817105544

areusch

@manupa-arm @kparzysz-quic @masahi thanks for the review! addressed your comments, ptal when you have time.

areusch · 2022-03-01T19:00:14Z

include/tvm/runtime/metadata.h

@@ -75,6 +75,13 @@ struct TVMMetadata {
  const struct TVMTensorInfo* outputs;
  /*! \brief Number of elements in `outputs` array. */
  int64_t num_outputs;
+  /*! \brief Memory Pools needed by the AOT run_model function.


changed to just "AOT main" since the function should probably eventually be renamed based on mod_name.

areusch · 2022-03-01T19:17:56Z

python/tvm/runtime/executor/__init__.py

+ - GraphExecutor, in python/tvm/contrib/graph_executor.py
+ - VM Executor, in python/tvm/runtime/vm.py
+
+TODO(areusch): Consolidate these into this module.


yeah that is a good point. added support here.

areusch · 2022-03-01T19:20:11Z

python/tvm/runtime/executor/aot_executor.py

+import numpy as np
+
+
+class AotModule(object):


i kind of find it easier to read the acronyms if we use CapWords, this also follows https://www.python.org/dev/peps/pep-0008/#class-names which is linked from numpydoc

areusch · 2022-03-01T19:22:44Z

python/tvm/runtime/executor/aot_executor.py

+        lib: tvm.runtime.Module = tvm.runtime.load_module("compiled_lib.so")
+        # Call the library factory function for default and create
+        # a new runtime.Module, wrap with graph module.
+        gmod = graph_executor.GraphModule(lib["default"](dev))


fixed, thanks!

areusch · 2022-03-01T19:23:34Z

src/relay/backend/aot_executor_codegen.cc

@@ -317,6 +327,16 @@ class AOTExecutorCodegen : public MixedModeVisitor {
    }
  }

+  void PushArgs(const Expr& expr, std::vector<tir::Var> sids, Array<PrimExpr>* args) {


oops, sorry kinda rough. changed to pass-by-const-ref and pointer per c++ style guide.

areusch · 2022-03-02T06:57:07Z

src/target/source/codegen_source_base.h

+ *        module.
+ * \param target_module The main TIR-lowered internal runtime module
+ * \param modules All the external modules that needs to be imported inside the metadata module(s).
+ * \param target The target that all the modules are compiled for


areusch · 2022-03-02T06:57:22Z

src/target/source/source_module.cc

+      address_.push_back(i_str.str());
+      Visit(nullptr, &metadata);
+      address_.pop_back();
+      //      ReflectionVTable::Global()->VisitAttrs(metadata.operator->(), this);


areusch · 2022-03-02T07:00:26Z

src/target/source/source_module.cc

+  lookup_func << "};" << std::endl;
+
+  auto mod = MetadataModuleCreate(metadata);
+  std::vector<String> func_names{"get_c_metadata"};


areusch · 2022-03-02T07:00:31Z

src/target/source/source_module.cc

+
+  auto mod = MetadataModuleCreate(metadata);
+  std::vector<String> func_names{"get_c_metadata"};
+  // definer.GetOutput() +


areusch · 2022-03-02T07:01:33Z

src/tir/transforms/legalize_packed_calls.cc

+                               {sid_array, 0, tir::builtin::kArrDeviceType, kDLCPU})));
+            new_stmts.push_back(tir::Evaluate(
+                tvm::tir::Call(DataType::Handle(), tvm::tir::builtin::tvm_struct_set(),
+                               {sid_array, 0, tir::builtin::kArrDeviceId, 0})));


these changes fixed the flaky stack bug where device allocations were using bad threadlocalstorage for workspace. the fix was to explicitly initialize the device_id and device_type fields of the created DLTensor. there are other fields we need to initialize as well, but which we don't use in the c-codgen generated code, at least not extensively.

masahi

LGTM modulo some unaddressed comments. But they can go into the next LLVM PR.

#10283 (comment)
#10283 (comment)

areusch · 2022-03-03T06:35:15Z

@manupa-arm @kparzysz-quic please take a look when you have a minute, CI is green now.

tmoreau89

LGTM - thanks @areusch for landing AoT executor !

tmoreau89 · 2022-03-03T17:51:30Z

Thank you @areusch @masahi @kparzysz-quic @manupa-arm for the reviews! The PR has been merged.

…che#10283) * Add memory pools to Metadata classes. * Move ShapeToJSON to utils. * Track returned TensorType from AOTExecutorCodegen. * Support calling Relay functions with Tuple. * Expand supported TIR calling conventions to work with C++ runtime. * Rename MetadataModule to ConstLoaderModule. * Add runtime AOT executor module. * Add AOT code-generation. * Add a runtime Module to mux between .text Metadata and live Metadata. * Move launch_param to namespace * Add test of c++ AOT. * Fix incongruity between kTvmRuntimeCrt constant * Expand ExecutorCodegenMetadata to include AOT runtime metadata. * commit cpp test * Make Metadata compile under C. * Ignore ephemeral metadata_module export_model_library_format. * This module does not need to be exported, since it is merely a C++ wrapper around get_c_metadata, and get_metadata is not used in C. * address manupa, kparszsyc, masahi comments. * further address comments * clang and python format * Fix broken test * Address lingering comments from masahi, kparszyzc

github-actions bot requested review from Mousius, kparzysz-quic, manupak and masahi February 17, 2022 03:26

areusch mentioned this pull request Feb 17, 2022

[Tracking Issue] Module-based Model Runtime Interface for AOT #10280

Open

5 tasks

masahi reviewed Feb 17, 2022

View reviewed changes

tests/python/relay/aot/test_cpp_aot.py Outdated Show resolved Hide resolved

github-actions bot requested a review from masahi February 17, 2022 06:00

areusch force-pushed the aot-c-metadata branch from 816c9ca to 512e3f4 Compare February 17, 2022 23:23

mehrdadh mentioned this pull request Feb 18, 2022

[runtime][Hexagon] AOTExecutor implementation for C Codegen #10311

Merged

areusch force-pushed the aot-c-metadata branch 2 times, most recently from 20fa726 to 9c02629 Compare February 18, 2022 23:40

masahi mentioned this pull request Feb 21, 2022

[runtime] Add Metadata classes for AOTExecutor #10282

Merged

areusch force-pushed the aot-c-metadata branch 2 times, most recently from f187f9f to 5f4d538 Compare February 23, 2022 06:06

areusch marked this pull request as ready for review February 23, 2022 06:13

areusch requested review from jroesch, slyubomirsky, icemelon, MarisaKirisame, ZihengJiang, yzhliu, vinx13, mbrookhart, jwfromm, zhiics, anijain2305, wweic, junrushao, tqchen and liangfu as code owners February 23, 2022 06:13

areusch force-pushed the aot-c-metadata branch from 928ac6f to ccc6c9c Compare February 26, 2022 01:37

masahi mentioned this pull request Feb 26, 2022

Add Module-Based Model Runtime Interface for AOT (support C++ runtime) #9697

Closed

kparzysz-quic approved these changes Feb 28, 2022

View reviewed changes

masahi reviewed Mar 1, 2022

View reviewed changes

manupak reviewed Mar 1, 2022

View reviewed changes

address manupa, kparszsyc, masahi comments.

c0be55c

github-actions bot requested review from kparzysz-quic, manupak and masahi March 1, 2022 19:59

masahi reviewed Mar 1, 2022

View reviewed changes

areusch added 2 commits March 2, 2022 10:16

further address comments

fda6d53

clang and python format

f32ba4b

areusch commented Mar 2, 2022

View reviewed changes

github-actions bot requested a review from masahi March 2, 2022 18:24

masahi approved these changes Mar 2, 2022

View reviewed changes

areusch added 2 commits March 2, 2022 16:21

Fix broken test

4f658a6

Address lingering comments from masahi, kparszyzc

2d42f22

github-actions bot requested a review from masahi March 3, 2022 00:22

tmoreau89 approved these changes Mar 3, 2022

View reviewed changes

tmoreau89 merged commit d721d32 into apache:main Mar 3, 2022

driazati mentioned this pull request Jul 14, 2022

TVM v0.9.0.rc0 Release Candidate Notes #12102

Closed

[runtime] AOTExecutor implementation and c target code-generator #10283

[runtime] AOTExecutor implementation and c target code-generator #10283

Conversation

areusch commented Feb 17, 2022 • edited

kparzysz-quic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masahi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masahi Mar 1, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manupak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masahi left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masahi Mar 1, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

areusch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

areusch commented Feb 17, 2022 •

edited

masahi Mar 1, 2022 •

edited

masahi left a comment •

edited

masahi Mar 1, 2022 •

edited