You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/04/06 16:43:38 UTC

[GitHub] [incubator-tvm] tqchen opened a new pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc

tqchen opened a new pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc
URL: https://github.com/apache/incubator-tvm/pull/5251
 
 
   Also moves dtype related handling to data_type.h
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc
URL: https://github.com/apache/incubator-tvm/pull/5251#discussion_r405095536
 
 

 ##########
 File path: include/tvm/runtime/packed_func.h
 ##########
 @@ -554,6 +512,10 @@ class TVMArgValue : public TVMPODValue_ {
       return std::string(value_.v_str);
     }
   }
+  operator tvm::runtime::String() const {
+    // directly use the std::string constructor for now.
+    return tvm::runtime::String(operator std::string());
 
 Review comment:
   https://github.com/apache/incubator-tvm/pull/5266

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] icemelon9 merged pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc

Posted by GitBox <gi...@apache.org>.
icemelon9 merged pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc
URL: https://github.com/apache/incubator-tvm/pull/5251
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc
URL: https://github.com/apache/incubator-tvm/pull/5251#discussion_r405046250
 
 

 ##########
 File path: include/tvm/runtime/packed_func.h
 ##########
 @@ -554,6 +512,10 @@ class TVMArgValue : public TVMPODValue_ {
       return std::string(value_.v_str);
     }
   }
+  operator tvm::runtime::String() const {
+    // directly use the std::string constructor for now.
+    return tvm::runtime::String(operator std::string());
 
 Review comment:
   @tqchen It happened to me that Line511 above failed for the check because the type_code_ for String is an object. Should we remove this and pass String objectref directly? Or do we need to handle String through FFI?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc
URL: https://github.com/apache/incubator-tvm/pull/5251#discussion_r405065312
 
 

 ##########
 File path: include/tvm/runtime/packed_func.h
 ##########
 @@ -554,6 +512,10 @@ class TVMArgValue : public TVMPODValue_ {
       return std::string(value_.v_str);
     }
   }
+  operator tvm::runtime::String() const {
+    // directly use the std::string constructor for now.
+    return tvm::runtime::String(operator std::string());
 
 Review comment:
   Ah, i see, we will need to add a patch, by checking if the result is kStr and run this, alternatively, use AsObjectRef<String>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] icemelon9 commented on issue #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc

Posted by GitBox <gi...@apache.org>.
icemelon9 commented on issue #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc
URL: https://github.com/apache/incubator-tvm/pull/5251#issuecomment-610017959
 
 
   Thanks @tqchen @zhiics 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on issue #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc

Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc
URL: https://github.com/apache/incubator-tvm/pull/5251#issuecomment-609907149
 
 
   cc @zhiics @wweic 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc
URL: https://github.com/apache/incubator-tvm/pull/5251#discussion_r405065312
 
 

 ##########
 File path: include/tvm/runtime/packed_func.h
 ##########
 @@ -554,6 +512,10 @@ class TVMArgValue : public TVMPODValue_ {
       return std::string(value_.v_str);
     }
   }
+  operator tvm::runtime::String() const {
+    // directly use the std::string constructor for now.
+    return tvm::runtime::String(operator std::string());
 
 Review comment:
   Ah, i see, good catch, we will need to add a patch, by checking if the result is kStr and run this, alternatively, use AsObjectRef<String>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5251: [RUNTIME] Auto conversion from str to runtime::String in PackedFUnc
URL: https://github.com/apache/incubator-tvm/pull/5251#discussion_r405092183
 
 

 ##########
 File path: include/tvm/runtime/packed_func.h
 ##########
 @@ -554,6 +512,10 @@ class TVMArgValue : public TVMPODValue_ {
       return std::string(value_.v_str);
     }
   }
+  operator tvm::runtime::String() const {
+    // directly use the std::string constructor for now.
+    return tvm::runtime::String(operator std::string());
 
 Review comment:
   so let's just `return AsObjectRef<tvm::runtime::String>()` for now?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services