[GitHub] [tvm] areusch commented on a change in pull request #8023: [AOT] Initial implementation of --unpacked-api

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

areusch commented on a change in pull request #8023:
URL: https://github.com/apache/tvm/pull/8023#discussion_r643315172



##########
File path: tests/python/relay/aot/test_crt_aot.py
##########
@@ -44,7 +44,8 @@
 
 
 @pytest.mark.parametrize("use_calculated_workspaces", [True, False])
-def test_conv_with_params(use_calculated_workspaces):
+@pytest.mark.parametrize("target_options", ["", "--unpacked-api"])

Review comment:
       it might make sense to specify --unpacked-api=0 rather than the implicit default behavior here, so that tests explicitly cover the packed-api case

##########
File path: include/tvm/runtime/module.h
##########
@@ -232,6 +232,8 @@ constexpr const char* tvm_param_prefix = "__tvm_param__";
 constexpr const char* tvm_lookup_linked_param = "_lookup_linked_param";
 /*! \brief The main AOT executor function */
 constexpr const char* tvm_run_func_prefix = "tvm__run_func";
+/*! \brief The entrypoint function to the generated network */

Review comment:
       let's at least update the comment here (and above, in the other constants) to explain the difference between `tvm_run_func_prefix`, `tvm_module_main`, and `tvm_entrypoint_name`. perhaps we should also qualify entrypoint (e.g. `tvm_unpacked_entrypoint_name`).

##########
File path: src/relay/backend/aot_executor_codegen.cc
##########
@@ -535,6 +551,8 @@ class AOTExecutorCodegen : public ExprVisitor {
   TargetsMap targets_;
   /*! \brief target host */
   Target target_host_;
+  /*! \brief untyped operators flag */

Review comment:
       could you explain this a bit more and specify polarity (e.g. true when abc thing should happen; false when other thing should happen)?

##########
File path: src/target/target_kind.cc
##########
@@ -219,6 +219,7 @@ TVM_REGISTER_TARGET_KIND("llvm", kDLCPU)
     .add_attr_option<Bool>("system-lib")
     .add_attr_option<String>("runtime")
     .add_attr_option<Bool>("link-params", Bool(false))
+    .add_attr_option<Bool>("unpacked-api", Bool(false))

Review comment:
       here i think we should not specify a default--this is why we see errors like https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/PR-8155/2/pipeline. These are identical to what we'll see when we migrate `unpacked-api` to a separate configuration space.
   
   ```
   Traceback (most recent call last):
     File "/usr/local/lib/python3.6/dist-packages/sphinx_gallery/gen_rst.py", line 480, in _memory_usage
       out = func()
     File "/usr/local/lib/python3.6/dist-packages/sphinx_gallery/gen_rst.py", line 465, in __call__
       exec(self.code, self.globals)
     File "/workspace/tutorials/get_started/auto_tuning_with_python.py", line 408, in <module>
       with autotvm.apply_history_best(tuning_option["tuning_records"]):
     File "/workspace/docs/../python/tvm/autotvm/task/dispatcher.py", line 201, in __init__
       self.load(records)
     File "/workspace/docs/../python/tvm/autotvm/task/dispatcher.py", line 229, in load
       for inp, res in records:
     File "/workspace/docs/../python/tvm/autotvm/record.py", line 212, in load_from_file
       ret = decode(row)
     File "/workspace/docs/../python/tvm/autotvm/record.py", line 157, in decode
       tgt = Target(str(tgt))
     File "/workspace/docs/../python/tvm/target/target.py", line 104, in __init__
       self.__init_handle_by_constructor__(_ffi_api.Target, target)
     File "tvm/_ffi/_cython/./object.pxi", line 126, in tvm._ffi._cy3.core.ObjectBase.__init_handle_by_constructor__
     File "tvm/_ffi/_cython/./packed_func.pxi", line 279, in tvm._ffi._cy3.core.ConstructorCall
     File "tvm/_ffi/_cython/./packed_func.pxi", line 257, in tvm._ffi._cy3.core.FuncCall
     File "tvm/_ffi/_cython/./packed_func.pxi", line 246, in tvm._ffi._cy3.core.FuncCall3
     File "tvm/_ffi/_cython/./base.pxi", line 163, in tvm._ffi._cy3.core.CALL
   ValueError: Traceback (most recent call last):
     5: TVMFuncCall
           at /workspace/src/runtime/c_runtime_api.cc:474
     4: tvm::runtime::PackedFunc::CallPacked(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*) const
           at /workspace/include/tvm/runtime/packed_func.h:1150
     3: std::function<void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)>::operator()(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*) const
           at /usr/include/c++/5/functional:2267
     2: std::_Function_handler<void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*), void (*)(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)>::_M_invoke(std::_Any_data const&, tvm::runtime::TVMArgs&&, tvm::runtime::TVMRetValue*&&)
           at /usr/include/c++/5/functional:1871
     1: tvm::TargetInternal::ConstructorDispatcher(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)
           at /workspace/src/target/target.cc:496
     0: tvm::Target::Target(tvm::runtime::String const&)
           at /workspace/src/target/target.cc:391
     File "/workspace/src/target/target.cc", line 391
   
   ValueError: Error when parsing target["unpacked-api"]: Cannot recognize 'unpacked-api'. Candidates are: link-params, system-lib, model, mfloat-abi, mcpu, tag, mattr, runtime, device, keys, libs, host, mtriple. Target creation from string failed: llvm -keys=cpu -link-params=0 -unpacked-api=0
   ```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org