gemini-code-assist[bot] commented on code in PR #18846:
URL: https://github.com/apache/tvm/pull/18846#discussion_r2867447754
##########
python/tvm/relax/op/distributed/distributed.py:
##########
@@ -99,7 +96,9 @@ def call_tir_local_view(
ret: Call
A call node for the call_tir_local_view operator.
"""
- if isinstance(args, Expr) and not isinstance(args, RxTuple): # type:
ignore
+ if isinstance(args, tuple | list):
+ args = RxTuple(list(args))
Review Comment:
When `args` is a `tuple` or `list`, its elements are not being converted to
`relax.Expr` before being passed to `RxTuple`. This can lead to a `TypeError`
if `args` contains non-Expr elements like integers or floats. You can use
`convert_to_expr` to handle this conversion, which will also create the
`relax.Tuple`. You will also need to add `from ...utils import convert_to_expr`
at the top of the file.
```suggestion
args = convert_to_expr(args)
```
##########
python/tvm/script/ir_builder/relax/distributed/ir.py:
##########
@@ -82,7 +80,9 @@ def call_tir(
if isinstance(func, str):
func = ExternFunc(func)
- if isinstance(args, Expr) and not isinstance(args, RxTuple): # type:
ignore
+ if isinstance(args, tuple | list):
+ args = RxTuple(list(args))
Review Comment:
When `args` is a `tuple` or `list`, its elements are not being converted to
`relax.Expr` before being passed to `RxTuple`. This can lead to a `TypeError`
if `args` contains non-Expr elements like integers or floats. You can use
`convert_to_expr` to handle this conversion, which will also create the
`relax.Tuple`. You will also need to add `from tvm.relax.utils import
convert_to_expr` at the top of the file.
```suggestion
args = convert_to_expr(args)
```
##########
python/tvm/relax/op/base.py:
##########
@@ -76,7 +76,9 @@ def _wrap_inline_arg_tuple(args) -> Expr:
in-line relax Tuple.
"""
- if (
+ if isinstance(args, tuple | list):
+ return tvm.relax.Tuple([convert_to_expr(a) if not isinstance(a, Expr)
else a for a in args])
Review Comment:
The list comprehension can be simplified. The `convert_to_expr` function
already handles the case where the input is an `Expr` by returning it as is, so
the `if not isinstance(a, Expr)` check is redundant.
```suggestion
return tvm.relax.Tuple([convert_to_expr(a) for a in args])
```
##########
python/tvm/relax/op/base.py:
##########
@@ -782,6 +783,10 @@ def call_pure_packed(
func = func.global_symbol
op = ExternFunc(func)
+ args = tuple(
+ convert_to_expr(a) if isinstance(a, int | float | str | tuple | list)
else a
+ for a in args
+ )
Review Comment:
Similar to a previous comment, this argument conversion logic can be
simplified. `convert_to_expr` is idempotent for `Expr` inputs, so you can apply
it to all arguments unconditionally.
```python
args = tuple(convert_to_expr(a) for a in args)
```
##########
python/tvm/relax/op/unary.py:
##########
@@ -546,6 +545,10 @@ def clip(x: Expr, min: Expr, max: Expr) -> Expr:
result : relax.Expr
The computed result.
"""
+ if not isinstance(min, Expr):
+ min = convert_to_expr(min)
+ if not isinstance(max, Expr):
+ max = convert_to_expr(max)
Review Comment:
This conversion logic can be simplified. `convert_to_expr` is idempotent on
`Expr` inputs, so the `isinstance` checks are not necessary.
```suggestion
min = convert_to_expr(min)
max = convert_to_expr(max)
```
##########
python/tvm/script/ir_builder/relax/ir.py:
##########
@@ -485,6 +488,9 @@ def call_py_func(
call: Call
The created Relax Call for call_py_func operator.
"""
+ args = py_tuple(
+ convert_to_expr(a) if isinstance(a, _CONVERTIBLE_TYPES) else a for a
in args
+ )
Review Comment:
Similar to a previous comment, this argument conversion logic can be
simplified. `convert_to_expr` is idempotent for `Expr` inputs, so you can apply
it to all arguments unconditionally.
```python
args = py_tuple(convert_to_expr(a) for a in args)
```
##########
python/tvm/relax/op/memory/memory.py:
##########
@@ -94,12 +94,13 @@ def alloc_tensor(
"""
if isinstance(offset, int):
offset = PrimValue(offset)
+ if not isinstance(shape, Expr):
+ shape = convert_to_expr(shape)
Review Comment:
This check is redundant because `convert_to_expr` already handles `Expr`
inputs by returning them as is. You can simplify this by calling
`convert_to_expr` unconditionally.
```python
shape = convert_to_expr(shape)
```
##########
python/tvm/relax/op/index.py:
##########
@@ -101,6 +100,11 @@ def strided_slice(
strided_slice require the input `begin`, `end` and `strides` to have the
same length as `axes`.
"""
+ axes = convert_to_expr(axes) if not isinstance(axes, Expr) else axes
+ begin = convert_to_expr(begin) if not isinstance(begin, Expr) else begin
+ end = convert_to_expr(end) if not isinstance(end, Expr) else end
+ if strides is not None and not isinstance(strides, Expr):
+ strides = convert_to_expr(strides)
Review Comment:
This conversion logic can be simplified. `convert_to_expr` is idempotent on
`Expr` inputs, so the `isinstance` checks are not necessary.
```suggestion
axes = convert_to_expr(axes)
begin = convert_to_expr(begin)
end = convert_to_expr(end)
if strides is not None:
strides = convert_to_expr(strides)
```
##########
python/tvm/relax/op/vm/vm.py:
##########
@@ -134,6 +135,8 @@ def call_tir_dyn(func: Expr, args: Tuple) -> Call:
result : Call
A relax Call to call_tir_dyn.
"""
+ if not isinstance(func, Expr):
+ func = convert_to_expr(func)
Review Comment:
This check is redundant because `convert_to_expr` already handles `Expr`
inputs by returning them as is. You can simplify this by calling
`convert_to_expr` unconditionally.
```python
func = convert_to_expr(func)
```
##########
python/tvm/relax/op/base.py:
##########
@@ -731,6 +729,10 @@ def call_inplace_packed(
func = func.global_symbol
op = ExternFunc(func)
+ args = tuple(
+ convert_to_expr(a) if isinstance(a, int | float | str | tuple | list)
else a
+ for a in args
+ )
Review Comment:
This argument conversion logic can be simplified. `convert_to_expr` handles
`Expr` inputs by returning them directly, so you don't need to conditionally
call it. Applying it to all arguments is simpler and has the same effect.
```python
args = tuple(convert_to_expr(a) for a in args)
```
##########
python/tvm/relax/op/memory/memory.py:
##########
@@ -50,6 +49,8 @@ def alloc_storage(
result : Call
A relax Call, which gets the allocated storage.
"""
+ if not isinstance(size, Expr):
+ size = convert_to_expr(size)
Review Comment:
This check is redundant because `convert_to_expr` already handles `Expr`
inputs by returning them as is. You can simplify this by calling
`convert_to_expr` unconditionally.
```python
size = convert_to_expr(size)
```
##########
python/tvm/relax/op/vm/vm.py:
##########
@@ -50,6 +49,8 @@ def alloc_storage(
result : Call
A relax Call, which gets the allocated storage.
"""
+ if not isinstance(shape, Expr):
+ shape = convert_to_expr(shape)
Review Comment:
This check is redundant because `convert_to_expr` already handles `Expr`
inputs by returning them as is. You can simplify this by calling
`convert_to_expr` unconditionally.
```python
shape = convert_to_expr(shape)
```
##########
python/tvm/relax/op/vm/vm.py:
##########
@@ -94,6 +94,8 @@ def alloc_tensor(
"""
if isinstance(offset, int):
offset = PrimValue(offset)
+ if not isinstance(shape, Expr):
+ shape = convert_to_expr(shape)
Review Comment:
This check is redundant because `convert_to_expr` already handles `Expr`
inputs by returning them as is. You can simplify this by calling
`convert_to_expr` unconditionally.
```python
shape = convert_to_expr(shape)
```
##########
python/tvm/script/ir_builder/relax/ir.py:
##########
@@ -428,6 +429,9 @@ def call_packed(
The created Relax Call
"""
op = ExternFunc(func)
+ args = py_tuple(
+ convert_to_expr(a) if isinstance(a, _CONVERTIBLE_TYPES) else a for a
in args
+ )
Review Comment:
This argument conversion logic can be simplified. `convert_to_expr` handles
`Expr` inputs by returning them directly, and also handles all types in
`_CONVERTIBLE_TYPES`, so you don't need to conditionally call it. Applying it
to all arguments is simpler and has the same effect.
```python
args = py_tuple(convert_to_expr(a) for a in args)
```
--
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]
