Lunderberg commented on code in PR #16183:
URL: https://github.com/apache/tvm/pull/16183#discussion_r1410897204
##########
include/tvm/runtime/packed_func.h:
##########
@@ -2129,6 +2315,42 @@ struct PackedFuncValueConverter<::tvm::runtime::String> {
}
};
+template <typename T>
+struct PackedFuncValueConverter<Array<T>> {
+ static Array<T> From(const TVMArgValue& val) {
+ auto untyped_array = val.AsObjectRef<Array<ObjectRef>>();
+
+ // Attempt to convert each item of the array into the desired
+ // type. If the items do not require a conversion, no copies are
+ // made.
+ return untyped_array.Map([](ObjectRef item) {
+ // The TVMArgValue is intentionally defined through
Review Comment:
Brainstorming, I think we could avoid the round trip through `TVMArgValue`
and `TVMRetValue` by instead changing the signature in
`PackedFuncValueConverter` so it could make recursive calls on the `ObjectRef`
directly.
```c++
template<typename T>
struct PackedFuncValueConverter<Array<T>> {
// Current API, value converter converts from TVMArgValue or
// TVMRetValue. The From() function is called whenever converting
// to Array<T>.
static Array<T> From(const TVMArgValue& val) { ... }
};
template<typename T>
struct PackedFuncValueConverter<Array<T>> {
// Proposed API, value converter converts from the value contained
// within the TVMArgValue or TVMRetValue. The From() function is
// called whenever converting to T, if the contained value is of an
// appropriate type.
//
// Exposing the individual overloads would allow them to be called
// without first wrapping in `TVMArgValue` or `TVMRetValue`.
static runtime::String From(ObjectRef obj) { ... }
};
// For types that are convertible from primitives, an overload
// accepting a primitive argument can be defined.
template<>
struct PackedFuncValueConverter<String> {
static String From(const char* prim) { ... }
static String From(const TVMByteArray* arr) { ... }
}
// When delegating to PackedFuncValueConverter, the existence of an
// overload can be checked at C++ compile-time, so the performance
// should be equivalent to the current checks.
inline TVMArgValue::operator T() const {
switch(type_code_) {
case kTVMArgInt:
// If the TVMArgValue holds an integer, and
// PackedFuncValueConverter defines a conversion from integer to
// T, then run the converter.
if constexpr(std::is_invocable_v<PackedFuncValueConverter<T>::From,
int64_t>) {
return PackedFuncValueConverter<T>::From(value_.v_int64);
}
break;
...
}
LOG(FATAL) << "Error message here";
}
```
I think such a change to `PackedFuncValueConverter` would be best done in a
follow-up PR, as this one is already on the larger side.
--
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]
