[GitHub] [tvm] huajsj commented on a change in pull request #9243: Add runtime.ModuleGetFormat method enabling export of BYOC generated sources which require a .cpp/.cc file extension

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

huajsj commented on a change in pull request #9243:
URL: https://github.com/apache/tvm/pull/9243#discussion_r727681973



##########
File path: python/tvm/runtime/module.py
##########
@@ -185,6 +185,11 @@ def type_key(self):
         """Get type key of the module."""
         return _ffi_api.ModuleGetTypeKey(self)
 
+    @property
+    def format(self):
+        """Get format of the module."""

Review comment:
       Get the format of the module.

##########
File path: python/tvm/runtime/module.py
##########
@@ -412,7 +418,9 @@ def export_library(self, file_name, fcompile=None, addons=None, workspace_dir=No
                 else:
                     assert module.type_key == "c"
                     object_format = "c"
-                    if "cc" in kwargs:
+                    if module.format in ["cc", "cpp"]:
+                        object_format = module.format
+                    elif "cc" in kwargs:
                         if kwargs["cc"] == "nvcc":
                             object_format = "cu"

Review comment:
       When "kwargs["cc"] == "nvcc" , for the format ["c", "cc", "cpp"] the object_format should either keep original format or use "cu" format, but currently logic distinguished ["c"] and ["cc", "cpp"] that seems not make sense. logic should like following.
   ```
   assert (module.format in ["c", "cc", "cpp"]) .f"<error message>"
   object_format = module.format
   if "cc" in kwargs:
       if kwargs["cc"] == "nvcc":
           object_format = "cu"
   ```

##########
File path: python/tvm/runtime/module.py
##########
@@ -402,7 +407,8 @@ def export_library(self, file_name, fcompile=None, addons=None, workspace_dir=No
         for index, module in enumerate(modules):
             if fcompile is not None and hasattr(fcompile, "object_format"):
                 if module.type_key == "c":
-                    object_format = "c"
+                    assert module.format in ["c", "cc", "cpp"]

Review comment:
       assert with error message?

##########
File path: include/tvm/runtime/module.h
##########
@@ -156,6 +156,11 @@ class TVM_DLL ModuleNode : public Object {
    * \return Possible source code when available.
    */
   virtual std::string GetSource(const std::string& format = "");
+  /*!
+   * \brief Get the format of module, when available.

Review comment:
       Get the format of 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