tqchen commented on code in PR #16183:
URL: https://github.com/apache/tvm/pull/16183#discussion_r1409884862


##########
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:
   add quick common case short cut, and avoid arg setter conversion logic
   ```c++
   if (auto* ptr = item->as<T::ContainerType>()) {
      return GetRef<T>(ptr);
   } 
   ```



##########
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:
   the main reason is that round trip through to TVMArg and TVMRet variant have 
cost of switching etc, and we should optimize for most freq path



##########
include/tvm/runtime/packed_func.h:
##########
@@ -2129,6 +2315,42 @@ struct PackedFuncValueConverter<::tvm::runtime::String> {
   }
 };
 
+template <typename T>
+struct PackedFuncValueConverter<Array<T>> {

Review Comment:
   We should likely check some of those cases, and update quite a few of them 
to `Array<runtime::Int>`. But agree some might be useful especially for the 
mixed symbolic shape  and static shape case



##########
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
+      // `TVMArgsSetter`, rather than defining it with
+      // `value.data_ = item.get();` and type code
+      // `kTVMObjectHandle`.  `TVMArgsSetter::operator()` includes
+      // special handling for unwrapping boxed primitives,
+      // PackedFunc, runtime::Module, etc, which should be checked
+      // before delegating to the array element's
+      // PackedFuncValueConverter implementation.
+      TVMValue value;
+      int type_code;
+      TVMArgsSetter setter(&value, &type_code);
+      setter(0, item);
+      TVMArgValue arg(value, type_code);
+      return PackedFuncValueConverter<T>::From(arg);
+    });
+  }
+  static Array<T> From(const TVMRetValue& val) {
+    auto untyped_array = val.AsObjectRef<Array<ObjectRef>>();
+
+    return untyped_array.Map([](ObjectRef item) {
+      TVMRetValue item_val;
+      item_val = std::move(item);

Review Comment:
   have same quick short cut 
   ```c++
   if (auto* ptr = item->as<T::ContainerType>()) {
      return GetRef<T>(ptr);
   } 
   ```
   
   the fallback conversion can remain the same?



##########
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
+      // `TVMArgsSetter`, rather than defining it with
+      // `value.data_ = item.get();` and type code
+      // `kTVMObjectHandle`.  `TVMArgsSetter::operator()` includes
+      // special handling for unwrapping boxed primitives,
+      // PackedFunc, runtime::Module, etc, which should be checked
+      // before delegating to the array element's
+      // PackedFuncValueConverter implementation.

Review Comment:
   do not quite get what we are intending to handle, maybe state we would like 
to allow conversion to classes like PrimExpr, so would like to trigger  that



-- 
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]

Reply via email to