gemini-code-assist[bot] commented on code in PR #18611:
URL: https://github.com/apache/tvm/pull/18611#discussion_r2646509555


##########
python/tvm/script/parser/relax/parser.py:
##########
@@ -254,7 +254,7 @@ def find_decorator_annotation(node: doc.FunctionDef, 
annotation: str, default: b
     """
     # look for the named argument in the function decorator
     for dec in node.decorator_list:
-        if not isinstance(dec, doc.Call) or dec.func.attr != "function":
+        if not isinstance(dec, doc.Call) or dec.func.attr != "tvm.function":

Review Comment:
   ![critical](https://www.gstatic.com/codereviewagent/critical.svg)
   
   The decorator name is `function`, not `tvm.function`. The `dec.func.attr` 
should be compared against `'function'`. This change will break parsing of 
`@R.function` decorators.
   
   ```suggestion
           if not isinstance(dec, doc.Call) or dec.func.attr != "function":
   ```



##########
python/tvm/contrib/rocm.py:
##########
@@ -227,7 +227,7 @@ def have_matrixcore(compute_version=None):
     return False
 
 
-@tvm_ffi.register_global_func("tvm_callback_rocm_get_arch")
+@tvm_ffi.register_global_func("tvm.tvm_callback_rocm_get_arch")

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The new function name `tvm.tvm_callback_rocm_get_arch` seems redundant. To 
improve clarity, consider renaming it to `tvm.callback_rocm_get_arch`.
   
   ```suggestion
   @tvm_ffi.register_global_func("tvm.callback_rocm_get_arch")
   ```



##########
python/tvm/contrib/nvcc.py:
##########
@@ -313,14 +313,14 @@ def find_nvshmem_paths() -> Tuple[str, str]:
     raise RuntimeError("\n".join(error_message))
 
 
-@tvm_ffi.register_global_func
+@tvm_ffi.register_global_func("tvm.tvm_callback_cuda_compile")
 def tvm_callback_cuda_compile(code, target):  # pylint: disable=unused-argument

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The new function name `tvm.tvm_callback_cuda_compile` seems redundant. To 
improve clarity and consistency, consider renaming it to 
`tvm.callback_cuda_compile` and also renaming the Python function to 
`callback_cuda_compile`.
   
   ```suggestion
   @tvm_ffi.register_global_func("tvm.callback_cuda_compile")
   def callback_cuda_compile(code, target):  # pylint: disable=unused-argument
   ```



##########
python/tvm/contrib/nvcc.py:
##########
@@ -313,14 +313,14 @@ def find_nvshmem_paths() -> Tuple[str, str]:
     raise RuntimeError("\n".join(error_message))
 
 
-@tvm_ffi.register_global_func
+@tvm_ffi.register_global_func("tvm.tvm_callback_cuda_compile")
 def tvm_callback_cuda_compile(code, target):  # pylint: disable=unused-argument
     """use nvcc to generate fatbin code for better optimization"""
     ptx = compile_cuda(code, target_format="fatbin")
     return ptx
 
 
-@tvm_ffi.register_global_func("tvm_callback_libdevice_path")
+@tvm_ffi.register_global_func("tvm.tvm_callback_libdevice_path")

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The new function name `tvm.tvm_callback_libdevice_path` seems redundant. To 
improve clarity, consider renaming it to `tvm.callback_libdevice_path`.
   
   ```suggestion
   @tvm_ffi.register_global_func("tvm.callback_libdevice_path")
   ```



##########
python/tvm/testing/utils.py:
##########
@@ -1629,7 +1629,7 @@ def fixture(func=None, *, cache_return_value=False):
     # Deliberately at function scope, so that caching can track how
     # many times the fixture has been used.  If used, the cache gets
     # cleared after the fixture is no longer needed.
-    scope = "function"
+    scope = "tvm.function"

Review Comment:
   ![critical](https://www.gstatic.com/codereviewagent/critical.svg)
   
   The scope of a pytest fixture has been changed to `tvm.function`, which is 
not a valid scope. The valid scopes are 'function', 'class', 'module', 
'package', or 'session'. This change will break pytest fixtures. Please revert 
it to 'function'.
   
   ```suggestion
       scope = "function"
   ```



##########
python/tvm/script/parser/relax/__init__.py:
##########
@@ -42,7 +42,7 @@
     "Shape",
     "Tensor",
     "Tuple",
-    "function",
+    "tvm.function",

Review Comment:
   ![critical](https://www.gstatic.com/codereviewagent/critical.svg)
   
   The name `tvm.function` is not a valid Python identifier and cannot be 
exported in `__all__`. This will cause an `ImportError`. Please revert this to 
`function`.
   
   ```suggestion
       "function",
   ```



##########
apps/ios_rpc/tests/ios_rpc_test.py:
##########
@@ -39,7 +39,7 @@
 
 
 # override metal compiler to compile to iphone
[email protected]_global_func("tvm_callback_metal_compile")
[email protected]_global_func("tvm.tvm_callback_metal_compile")

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The new function name `tvm.tvm_callback_metal_compile` seems redundant. To 
improve clarity and consistency with the new naming scheme, consider renaming 
it to `tvm.callback_metal_compile`. This would make the naming scheme more 
consistent by avoiding the repeated `tvm` prefix.
   
   ```suggestion
   @tvm.register_global_func("tvm.callback_metal_compile")
   ```



##########
python/tvm/contrib/rocm.py:
##########
@@ -123,7 +123,7 @@ def callback_rocm_link(obj_bin):
     return cobj_bin
 
 
-@tvm_ffi.register_global_func("tvm_callback_rocm_bitcode_path")
+@tvm_ffi.register_global_func("tvm.tvm_callback_rocm_bitcode_path")

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The new function name `tvm.tvm_callback_rocm_bitcode_path` seems redundant. 
To improve clarity, consider renaming it to `tvm.callback_rocm_bitcode_path`.
   
   ```suggestion
   @tvm_ffi.register_global_func("tvm.callback_rocm_bitcode_path")
   ```



##########
python/tvm/contrib/rocm.py:
##########
@@ -99,7 +99,7 @@ def rocm_link(in_file, out_file, lld=None):
         raise RuntimeError(msg)
 
 
-@tvm_ffi.register_global_func("tvm_callback_rocm_link")
+@tvm_ffi.register_global_func("tvm.tvm_callback_rocm_link")

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The new function name `tvm.tvm_callback_rocm_link` seems redundant. To 
improve clarity, consider renaming it to `tvm.callback_rocm_link`.
   
   ```suggestion
   @tvm_ffi.register_global_func("tvm.callback_rocm_link")
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to