You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "echuraev (via GitHub)" <gi...@apache.org> on 2023/01/30 09:21:46 UTC

[GitHub] [tvm] echuraev opened a new pull request, #13868: [OpenCL] Implement save/load pre-compiled programs

echuraev opened a new pull request, #13868:
URL: https://github.com/apache/tvm/pull/13868

   Using pre-compiled programs might significantly improve inference time of the first run.
   
   - Added methods `SupportPreCompiledPrograms` which reports if the module supports using pre-compiled programs.
   - Method `GetPreCompiledPrograms` returns string with bytes of pre-compiled programs.
   - Method `SetPreCompiledPrograms` allows user to pass pre-compiled programs to the module.


-- 
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] tvm-bot commented on pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #13868:
URL: https://github.com/apache/tvm/pull/13868#issuecomment-1408248584

   <!---bot-comment-->
   
   Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
   
   <!--bot-comment-ccs-start-->
    * cc @elvin-n <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
   
   <sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>


-- 
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] srkreddy1238 merged pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "srkreddy1238 (via GitHub)" <gi...@apache.org>.
srkreddy1238 merged PR #13868:
URL: https://github.com/apache/tvm/pull/13868


-- 
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] echuraev commented on pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on PR #13868:
URL: https://github.com/apache/tvm/pull/13868#issuecomment-1408250085

   cc: @tqchen, @csullivan, @srkreddy1238 


-- 
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] elvin-n commented on pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "elvin-n (via GitHub)" <gi...@apache.org>.
elvin-n commented on PR #13868:
URL: https://github.com/apache/tvm/pull/13868#issuecomment-1411657638

   Supporting the idea of caching data, I am against of API introducing work with file system. We can provide serialize interfaces that users can implement and feed to TVM API or we can provide load/store functions operating with binary in memory, but it must not be file system dependent API.
   
   


-- 
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] echuraev commented on pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on PR #13868:
URL: https://github.com/apache/tvm/pull/13868#issuecomment-1408798038

   @tvm-bot rerun


-- 
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] srkreddy1238 commented on pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "srkreddy1238 (via GitHub)" <gi...@apache.org>.
srkreddy1238 commented on PR #13868:
URL: https://github.com/apache/tvm/pull/13868#issuecomment-1411761781

   > Supporting the idea of caching data, I am against of API introducing work with file system. We can provide serialize interfaces that users can implement and feed to TVM API or we can provide load/store functions operating with binary in memory, but it must not be file system dependent API.
   
   You mean, TVM will prepare in memory serialized blob for all the cache, application is responsible for retrieving followed by storing on their own and later user will input the the binary blob back to TVM (similar to binary input for load_params) ?
   


-- 
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 diff in pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on code in PR #13868:
URL: https://github.com/apache/tvm/pull/13868#discussion_r1090670535


##########
include/tvm/runtime/module.h:
##########
@@ -192,6 +192,27 @@ class TVM_DLL ModuleNode : public Object {
   /*! \return The module it imports from */
   const std::vector<Module>& imports() const { return imports_; }
 
+  /*!
+   * \brief Returns true if this module supports building from pre-compiled programs.
+   *
+   * The default implementation returns false.
+   */
+  virtual bool SupportPreCompiledPrograms() const { return false; }
+
+  /*!
+   * \brief Pass pre-compiled programs which module will use to speed up compilation time.
+   * \param bytes string with bytes of pre-compiled programs.
+   */

Review Comment:
   Given this is OpenCL specific, let us keep it local using existing runtime.Module mechanism.
   
   We can support pre-compiled binary directly in OpenCLModule serialization. So the main goal is to enable such serializable as part of opencl binary.
   
   We can have a separate load_binary key suffix to enable backward compatibility.



-- 
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] srkreddy1238 commented on pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "srkreddy1238 (via GitHub)" <gi...@apache.org>.
srkreddy1238 commented on PR #13868:
URL: https://github.com/apache/tvm/pull/13868#issuecomment-1411536112

   @echuraev thanks for this feature.
   
   I too had a similar problem statement with CLML tuning cache management at runtime which need to be generated once and reused later. In CLML, there existed multiple sub graphs related to each tvm module and for now the tuning cache is indexed by subgraph symbol and are stored under one file by serializing them through ```DMLC::Strm```.  This works by maintaining one file per tvm module. This approach adds additional overhead for the user to maintain and specify the tuning cache for each tvm module.
   
   I think we have a generalized problem statement here where there is a need of cache management for the tvm runtime.
   
   Probably we could come up with an unified approach where
   - There will be  a tvm_runtime cache specified by environment variale  or graph_runtime api interface.
   - Each tvm compiled module will have a unique hash key generated at compile time and is accessible from tvm module interface.
   - A new file utility interface to load/store the binary blobs generated by any runtime as a key & value pair.
   
   The flow would be like 
   
   1: Runtime (OpenCL/CLML ..etc.) will form a key (concatenating [ tvm_module hash + runtime + purpose ...etc.] )
   2: Try to fetch from runtime cache.
   3: If not exist regenerate and save into cache.
   4: Use the stored cache
   
   This will simplify and minimize the end user hassle to a level of just specify a cache folder and relax.
   
   
   In the implementation side we have
   
   - tvm Module passing a unique key from compilation to runtime probably via grpah_json.
   - Cache API to serialize and load/store the key, value paired binary blobs.
   - Runtime specific changes to use this cache interface.
   


-- 
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] echuraev commented on a diff in pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on code in PR #13868:
URL: https://github.com/apache/tvm/pull/13868#discussion_r1090724562


##########
include/tvm/runtime/module.h:
##########
@@ -192,6 +192,27 @@ class TVM_DLL ModuleNode : public Object {
   /*! \return The module it imports from */
   const std::vector<Module>& imports() const { return imports_; }
 
+  /*!
+   * \brief Returns true if this module supports building from pre-compiled programs.
+   *
+   * The default implementation returns false.
+   */
+  virtual bool SupportPreCompiledPrograms() const { return false; }
+
+  /*!
+   * \brief Pass pre-compiled programs which module will use to speed up compilation time.
+   * \param bytes string with bytes of pre-compiled programs.
+   */

Review Comment:
   @tqchen thank you or your feedback! You mean that we need to update `SaveToBinary` method and serialize pre-compiled programs in the library with the model, then during the loading we can load these pre-compiled programs and create OpenCL programs from them? Did I understand correctly?
   
   Unfortunately, we cannot get pre-compiled programs during the model build. Because OpenCL kernels are compiled on the first run of the OpenCL runtime and the format of the pre-compiled programs is hardware specific. So we can get the pre-compiled programs only on the target device in the runtime. The main idea of this feature that the end user can significantly decrease the built time of his model in a real application. For example, on the installation phase, the user can get pre-compiled programs and write them somewhere on the device. And on the next run, these pre-compiled kernels will be used in this application. I have updated `rtvm` application to demonstrate this scenario.



-- 
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] srkreddy1238 commented on pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "srkreddy1238 (via GitHub)" <gi...@apache.org>.
srkreddy1238 commented on PR #13868:
URL: https://github.com/apache/tvm/pull/13868#issuecomment-1412126444

   User management will impose additional challenge.
   
   Every time the application should retrieve and save the same as we are unaware of change to the cache blob in the current module load. Or the application should query about a change due to the current run. I feel all these complicates the app interface.
   
   @echuraev cache implementation incur more changes outside cl runtime and can be handled outside this PR scope once there is an agreement on the  design.
   


-- 
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] echuraev commented on pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on PR #13868:
URL: https://github.com/apache/tvm/pull/13868#issuecomment-1413616618

   @tqchen could you please review this PR once again?


-- 
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] echuraev commented on a diff in pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on code in PR #13868:
URL: https://github.com/apache/tvm/pull/13868#discussion_r1094153020


##########
apps/cpp_rtvm/tvm_runner.cc:
##########
@@ -110,6 +113,35 @@ int TVMRunner::Load(void) {
   return 0;
 }
 
+/*!
+ * \brief Specify if the run programs should be dumped to binary and reused in the next runs.
+ * \param pathToDir Path to the existed directory where pre-compiled programs should be stored.
+ */
+void TVMRunner::UsePreCompiledPrograms(std::string pathToDir) {
+  if (r_run_was_called) {
+    LOG(INFO) << "TVMRunner UsePreCompiledPrograms: should be called before first run";
+    return;
+  }
+  if (!std::filesystem::exists(pathToDir))

Review Comment:
   Done



-- 
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] echuraev commented on pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on PR #13868:
URL: https://github.com/apache/tvm/pull/13868#issuecomment-1411804399

   @srkreddy1238 Thank you for your review! The idea of caching data looks promising, but I agree with @elvin-n that the TVM should provide only API for serializing objects to binary format but shouldn't work with file system. So user application should be responsible for writing/reading this serialized blob to/from disk.
   
   Do you think that caching mechanism should be implemented in this PR? I would prefer to see such implementation in separate PR.


-- 
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] echuraev commented on a diff in pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on code in PR #13868:
URL: https://github.com/apache/tvm/pull/13868#discussion_r1093024517


##########
include/tvm/runtime/module.h:
##########
@@ -192,6 +192,27 @@ class TVM_DLL ModuleNode : public Object {
   /*! \return The module it imports from */
   const std::vector<Module>& imports() const { return imports_; }
 
+  /*!
+   * \brief Returns true if this module supports building from pre-compiled programs.
+   *
+   * The default implementation returns false.
+   */
+  virtual bool SupportPreCompiledPrograms() const { return false; }
+
+  /*!
+   * \brief Pass pre-compiled programs which module will use to speed up compilation time.
+   * \param bytes string with bytes of pre-compiled programs.
+   */

Review Comment:
   Added functionality to call these functions by using method `GetFunction`.



-- 
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] srkreddy1238 commented on a diff in pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "srkreddy1238 (via GitHub)" <gi...@apache.org>.
srkreddy1238 commented on code in PR #13868:
URL: https://github.com/apache/tvm/pull/13868#discussion_r1094023001


##########
apps/cpp_rtvm/tvm_runner.cc:
##########
@@ -110,6 +113,35 @@ int TVMRunner::Load(void) {
   return 0;
 }
 
+/*!
+ * \brief Specify if the run programs should be dumped to binary and reused in the next runs.
+ * \param pathToDir Path to the existed directory where pre-compiled programs should be stored.
+ */
+void TVMRunner::UsePreCompiledPrograms(std::string pathToDir) {
+  if (r_run_was_called) {
+    LOG(INFO) << "TVMRunner UsePreCompiledPrograms: should be called before first run";
+    return;
+  }
+  if (!std::filesystem::exists(pathToDir))

Review Comment:
   ```rtvm``` tool is not specific to Adreno or OpenCL. It works for any target. Probably we could have a target check here.
   
   Also, Can the path be made command line arg for the tool ?



-- 
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 diff in pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on code in PR #13868:
URL: https://github.com/apache/tvm/pull/13868#discussion_r1090670535


##########
include/tvm/runtime/module.h:
##########
@@ -192,6 +192,27 @@ class TVM_DLL ModuleNode : public Object {
   /*! \return The module it imports from */
   const std::vector<Module>& imports() const { return imports_; }
 
+  /*!
+   * \brief Returns true if this module supports building from pre-compiled programs.
+   *
+   * The default implementation returns false.
+   */
+  virtual bool SupportPreCompiledPrograms() const { return false; }
+
+  /*!
+   * \brief Pass pre-compiled programs which module will use to speed up compilation time.
+   * \param bytes string with bytes of pre-compiled programs.
+   */

Review Comment:
   Would be great to not touch the general module interface. Instead, support pre-compiled binary directly in OpenCLModule serialization. So the main goal is to enable such serializable as part of opencl binary.
   
   We can have a separate load_binary key suffix to enable backward compatibility.



-- 
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 diff in pull request #13868: [OpenCL] Implement save/load pre-compiled programs

Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on code in PR #13868:
URL: https://github.com/apache/tvm/pull/13868#discussion_r1090869599


##########
include/tvm/runtime/module.h:
##########
@@ -192,6 +192,27 @@ class TVM_DLL ModuleNode : public Object {
   /*! \return The module it imports from */
   const std::vector<Module>& imports() const { return imports_; }
 
+  /*!
+   * \brief Returns true if this module supports building from pre-compiled programs.
+   *
+   * The default implementation returns false.
+   */
+  virtual bool SupportPreCompiledPrograms() const { return false; }
+
+  /*!
+   * \brief Pass pre-compiled programs which module will use to speed up compilation time.
+   * \param bytes string with bytes of pre-compiled programs.
+   */

Review Comment:
   Thank you @echuraev . In that case, perhaps it is worthwhile to have the OpenCLModule expose a special packed methods `__GetPreCompiledPrograms` (just like what GraphRuntimeExecutor will do) So you can expose them through PackedFuncs without having to change  the runtime.Module interface



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