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 2021/10/21 16:00:57 UTC

[GitHub] [tvm] csullivan opened a new pull request #9342: Support runtime defined function wrapping of library module packed functions

csullivan opened a new pull request #9342:
URL: https://github.com/apache/tvm/pull/9342


   Targets which compile directly to SO during codegen utilize `runtime.module.loadfile_so` at runtime to dynamically open these compiled binaries. This flow currently does not have any point of extension in which a runtime can customize the behavior of the underlying LibraryModuleNode. This PR exposes DSOLibrary and extends LibraryModuleNode to utilize a runtime defined PackedFunctionWrapper in order to specialize the calling convention of dynamically loaded function symbols.
   
   This allows a device runtime to then specify a custom DSOLibrary based loadfile implementation:
   
   ```C++
   TVM_REGISTER_GLOBAL("runtime.module.loadfile_hexagon").set_body([](TVMArgs args, TVMRetValue* rv) {
     auto n = make_object<DSOLibrary>();
     n->Init(args[0]);
     *rv = CreateModuleFromLibrary(n, make_object<hexagon::WrapPackedFunc>());
   });
   ```


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



[GitHub] [tvm] tqchen commented on a change in pull request #9342: Support runtime defined function wrapping of library module packed functions

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #9342:
URL: https://github.com/apache/tvm/pull/9342#discussion_r733835789



##########
File path: src/runtime/library_module.h
##########
@@ -65,6 +65,23 @@ class Library : public Object {
   // This is because we do not need dynamic type downcasting.
 };
 
+/*!
+ * \brief Default virtual functor that provides an interface to
+ * wrap a TVMBackendPackedCFunc. Virtual interface allows derivative
+ * runtime's that utilize a library module to to provide custom
+ * function wrapping. By default WrapPackedFunc is used.
+ */
+class PackedFuncWrapper : public Object {
+ public:

Review comment:
       maybe sufficient to use a std::function that passes in from CreateModuleFromLibrary to reduce the amount of the abstractions and indirections




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



[GitHub] [tvm] tqchen commented on a change in pull request #9342: Support runtime defined function wrapping of library module packed functions

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #9342:
URL: https://github.com/apache/tvm/pull/9342#discussion_r734563076



##########
File path: src/runtime/dso_library.h
##########
@@ -0,0 +1,93 @@
+/*
+ * 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 dso_module.h

Review comment:
       dso_library.h

##########
File path: src/runtime/library_module.h
##########
@@ -78,16 +78,24 @@ PackedFunc WrapPackedFunc(TVMBackendPackedCFunc faddr, const ObjectPtr<Object>&
  */
 void InitContextFunctions(std::function<void*(const char*)> fgetsymbol);
 
+/*!
+ * \brief Type alias for funcion to wrap a TVMBackendPackedCFunc.
+ */
+using PackedFuncWrapper =
+    std::function<PackedFunc(TVMBackendPackedCFunc, const ObjectPtr<Object>&)>;

Review comment:
       would be great to document the parameters and return values as part of the doc string.

##########
File path: src/runtime/dso_library.h
##########
@@ -0,0 +1,93 @@
+/*
+ * 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 dso_module.h

Review comment:
       depending on whether or not we need to expose this interface, we might want to put it back to the cc file to reduce the amount of internal interface. surface.




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



[GitHub] [tvm] csullivan commented on a change in pull request #9342: Support runtime defined function wrapping of library module packed functions

Posted by GitBox <gi...@apache.org>.
csullivan commented on a change in pull request #9342:
URL: https://github.com/apache/tvm/pull/9342#discussion_r733979316



##########
File path: src/runtime/library_module.h
##########
@@ -65,6 +65,23 @@ class Library : public Object {
   // This is because we do not need dynamic type downcasting.
 };
 
+/*!
+ * \brief Default virtual functor that provides an interface to
+ * wrap a TVMBackendPackedCFunc. Virtual interface allows derivative
+ * runtime's that utilize a library module to to provide custom
+ * function wrapping. By default WrapPackedFunc is used.
+ */
+class PackedFuncWrapper : public Object {
+ public:

Review comment:
       Good suggestion, [updated](https://github.com/apache/tvm/pull/9342/files#diff-c6e5039ebc3f0bccd4c79d58f05c3a7ac51e0719d23477d504c32a0ab1aa2312R81-R86).




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



[GitHub] [tvm] csullivan commented on a change in pull request #9342: Support runtime defined function wrapping of library module packed functions

Posted by GitBox <gi...@apache.org>.
csullivan commented on a change in pull request #9342:
URL: https://github.com/apache/tvm/pull/9342#discussion_r736055633



##########
File path: src/runtime/dso_library.h
##########
@@ -0,0 +1,93 @@
+/*
+ * 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 dso_module.h

Review comment:
       As Library is an abstraction that is already exposed, I decided to reduce the interface to a [helper](https://github.com/apache/tvm/pull/9342/files#diff-c6e5039ebc3f0bccd4c79d58f05c3a7ac51e0719d23477d504c32a0ab1aa2312R90-R96) that returns one instead.




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



[GitHub] [tvm] masahi merged pull request #9342: Support runtime defined function wrapping of library module packed functions

Posted by GitBox <gi...@apache.org>.
masahi merged pull request #9342:
URL: https://github.com/apache/tvm/pull/9342


   


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