Lunderberg commented on PR #15983:
URL: https://github.com/apache/tvm/pull/15983#issuecomment-1799131469
This arose when writing unit tests for access of a relax tuple. I expected
the following unit test to pass on `unity` HEAD.
```python
import tvm.testing
from tvm.script import relax as R
exec_mode = tvm.testing.parameter("bytecode", "compiled")
def test_vm_tuple_get_item(exec_mode):
@R.function(private=True)
def func(arg: R.Tuple([R.Prim("int64"), R.Prim("float32")])):
return arg[0]
mod = tvm.IRModule({"main": func})
target = tvm.target.Target("llvm", host="llvm")
ex = tvm.relax.build(mod, target, exec_mode=exec_mode)
vm = tvm.relax.VirtualMachine(ex, tvm.cpu())
res = vm["main"]((17, 42.5))
assert res == 17
```
The error occurs with the following steps:
1. When passed an argument, python tuples are converted to `tvm.ir.Array`.
2. In order to use `tvm.ir.Array`, the arguments must be converted to
`ObjectRef` subclasses. This converts `(17, 42.5)` into `tvm.ir.Array([
T.int64(17), T.float32(42.5) ])`.
3. The tuple is accessed with a relax VM builtin, and `T.int64(17)` is
stored in a relax VM register.
4. The `T.int64(17)` is type-checked as a `R.prim('int64')`, which uses
`TVMPODValue_::operator int64_t`.
5. The call to `operator int64_t` fails, as it is an `IntImm`, and uses
`TVMArgTypeCode::kTVMObjectHandle` and not `TVMArgTypeCode::kTVMArgInt`.
Any of these steps could be broken in order to avoid the error.
Fundamentally, the error occurs because we apply an automatic conversion (`int`
to `IntImm`), but do not automatically apply the reverse conversion (`IntImm`
to `int`). I believe that this PR, which changes step (5), is the best
resolution. Avoiding the error by updating any of the other steps has
drawbacks as described below.
1. Change the FFI conversion of python tuples to a runtime-only type, rather
than `tvm.ir.Array`. This would cause breakage for any FFI call that uses
`Array` at compile-time.
2. Change the internal type of `tvm.ir.Array` from `ObjectRef` to
`TVMArgValue`. This would allow FFI conversions from python tuples to `Array`,
but would break the usage of those `Array` objects.
3. Change the return of the `tuple_getitem` builtin to convert from `IntImm`
to `int` when providing a `TVMRetValue`. Possible, but would drop the dtype
information that may be required for later type-checks.
4. Change the type-checking of `R.Prim` in the VM to check for `IntImm`.
This would avoid the error in this case, but this function is used for
type-checking before calling external `PackedFunc` instances. Those calls
would throw the same error at a later point.
5. Change the `operator int64_t` implementation to check for an `IntImm`.
That is the change implemented in this PR, and handles any such conversion
required.
> Given the implementation complexity
Can you explain what you mean here? This doesn't seem any more complex than
the argument handling that we already do for FFI compatibility purposes.
--
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]
