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 2020/01/06 22:21:15 UTC

[GitHub] [incubator-tvm] tqchen opened a new pull request #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

tqchen opened a new pull request #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637
 
 
   Previously the signature of LibraryModule's PackedFunc does not support return value.
   This wasn't a limitation for our current usecase but could become one
   as we start to generate more interesting functions.
   
   This feature also start to get interesting as we move towards unified
   object protocol and start to pass object around.
   This PR enhances the function signature to allow return values.
   
   We also created two macros TVM_DLL_EXPORT_PACKED_FUNC and TVM_DLL_EXPORT_TYPED_FUNC
   to allow manual creation of functions that can be loaded by a LibraryModule.
   
   Examples are added in apps/dso_plugin_module.
   The change to TVMBackendPackedCFunc is backward compatible,
   as previous function will simply ignore the return value field.
   

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac edited a comment on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
comaniac edited a comment on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637#issuecomment-571367737
 
 
   Thanks for the PR. The macros are indeed helpful for customized runtimes and can reduce the developer efforts. I'll file another PR to make use of them in [this example](https://github.com/apache/incubator-tvm/blob/master/src/runtime/contrib/example_ext_runtime/example_ext_runtime.cc) after this one have been merged. 
   
   The change are LGTM. Since I may not be aware of some concerns in backend APIs, I am leaving it for others.
   
   btw, you may need to fix the failed cases in CI.

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen edited a comment on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637#issuecomment-571341450
 
 
   cc @comaniac @zhiics @wweic @yzhliu @ZihengJiang @FrozenGene 
   
   NOTE: the macros are also useful for bring your own backend codegen

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] wweic commented on a change in pull request #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
wweic commented on a change in pull request #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637#discussion_r363614164
 
 

 ##########
 File path: apps/dso_plugin_module/test_plugin_module.py
 ##########
 @@ -0,0 +1,47 @@
+# 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.
+import tvm
+import os
+
+def test_plugin_module():
+    curr_path = os.path.dirname(os.path.abspath(os.path.expanduser(__file__)))
+    mod = tvm.module.load(os.path.join(curr_path, "lib", "plugin_module.so"))
+    # NOTE: we need to make sure all managed resources returned
+    # from mod get destructed before mod get unloaded.
+    #
+    # Failure mode we want to prevent from:
+    # We retain an object X whose destructor is within mod.
+    # The program will segfault if X get destructed after mod,
+    # because the destructor function has already been unloadd.
 
 Review comment:
   ```suggestion
       # because the destructor function has already been unloaded.
   ```

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637#discussion_r363602628
 
 

 ##########
 File path: include/tvm/runtime/packed_func.h
 ##########
 @@ -877,6 +894,104 @@ class TVMRetValue : public TVMPODValue_ {
   }
 };
 
+/*!
+ * \brief Export a function with the PackedFunc signature
+ *        as a PackedFunc that can loaded by LibraryModule.
+ *
+ * \param ExportName The symbol name to be exported.
+ * \param Function The function with PackedFunc signature.
+ * \sa PackedFunc
+ *
+ * \code
+ *
+ * void AddOne_(TVMArgs args, TVMRetValue* rv) {
+ *   int value = args[0];
+ *   *rv = value + 1;
+ * }
+ * // Expose the function as "AddOne"
+ * TVM_DLL_EXPORT_PACKED_FUNC(AddOne, AddOne_);
+ *
+ * \endcode
+ */
+#define TVM_DLL_EXPORT_PACKED_FUNC(ExportName, Function)                \
+  extern "C" {                                                          \
+  TVM_DLL int ExportName(TVMValue* args,                                \
+                         int* type_code,                                \
+                         int num_args,                                  \
+                         TVMValue* out_value,                           \
+                         int* out_type_code) {                          \
+    try {                                                               \
+      ::tvm::runtime::TVMRetValue rv;                                   \
+      Function(::tvm::runtime::TVMArgs(                                 \
+          args, type_code, num_args), &rv);                             \
+      rv.MoveToCHost(out_value, out_type_code);                         \
+      return 0;                                                         \
+    } catch (const ::std::runtime_error& _except_) {                    \
+      TVMAPISetLastError(_except_.what());                              \
+      return -1;                                                        \
+    }                                                                   \
+  }                                                                     \
+  }
+
+/*!
+ * \brief Export typed function as a PackedFunc
+ *        that can loaded by LibraryModule.
 
 Review comment:
   ```suggestion
    *        that can loaded be by LibraryModule.
   ```

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac edited a comment on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
comaniac edited a comment on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637#issuecomment-571367737
 
 
   Thanks for the PR. The macros are indeed helpful for customized runtimes and can reduce the developer efforts. I'll file another PR to make use of them in [this example](https://github.com/apache/incubator-tvm/blob/master/src/runtime/contrib/example_ext_runtime/example_ext_runtime.cc) after this one have been merged. 
   
   The change are LGTM. Since I may not be aware of some concerns in backend APIs, I am leaving it for others.
   
   btw, you may need to fix the failure cases in CI.

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen merged pull request #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637
 
 
   

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637#issuecomment-571821707
 
 
   Thanks @wweic @ZihengJiang @zhiics 

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] comaniac commented on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
comaniac commented on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637#issuecomment-571367737
 
 
   Thanks for the PR. The macros are indeed helpful for customized runtimes and can reduce the developer efforts. I'll file another PR to make use of them in [this example](https://github.com/apache/incubator-tvm/blob/master/src/runtime/contrib/example_ext_runtime/example_ext_runtime.cc) after this one have been merged. 
   
   The change are LGTM. Since I may not be aware of some concerns in backend APIs, I am leaving it for others.
   

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637#issuecomment-571341450
 
 
   cc @comaniac @zhiics @wweic @yzhliu 

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637#discussion_r363602346
 
 

 ##########
 File path: include/tvm/runtime/packed_func.h
 ##########
 @@ -877,6 +894,104 @@ class TVMRetValue : public TVMPODValue_ {
   }
 };
 
+/*!
+ * \brief Export a function with the PackedFunc signature
+ *        as a PackedFunc that can loaded by LibraryModule.
 
 Review comment:
   ```suggestion
    *        as a PackedFunc that can be loaded by LibraryModule.
   ```

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen edited a comment on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val

Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #4637: [RUNTIME][DSO] Improve TVMBackendPackedCFunc to allow return val
URL: https://github.com/apache/incubator-tvm/pull/4637#issuecomment-571341450
 
 
   cc @comaniac @zhiics @wweic @yzhliu 
   
   NOTE: the macros are also useful for bring your own backend codegen

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


With regards,
Apache Git Services