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/07/24 07:35:45 UTC

[GitHub] [incubator-tvm] antinucleon opened a new pull request #6135: [runtime] add float for podvalue

antinucleon opened a new pull request #6135:
URL: https://github.com/apache/incubator-tvm/pull/6135


   Thanks for contributing to TVM!   Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
   


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



[GitHub] [incubator-tvm] tqchen commented on pull request #6135: [runtime] add float for podvalue

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6135:
URL: https://github.com/apache/incubator-tvm/pull/6135#issuecomment-663667292


   Given that we already have operator double, let us simply ignore the float and use the double as positional argument in the function signature(say TypedPackedFunc, then convert later?)


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



[GitHub] [incubator-tvm] antinucleon commented on a change in pull request #6135: [runtime] add float for podvalue

Posted by GitBox <gi...@apache.org>.
antinucleon commented on a change in pull request #6135:
URL: https://github.com/apache/incubator-tvm/pull/6135#discussion_r460156172



##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -371,6 +371,20 @@ class TVMPODValue_ {
     TVM_CHECK_TYPE_CODE(type_code_, kDLFloat);
     return value_.v_float64;
   }
+  operator float() const {
+    double tmp_v = 0;
+    if (type_code_ == kDLInt) {
+      tmp_v = static_cast<double>(value_.v_int64);
+      CHECK_LE(tmp_v, std::numeric_limits<float>::max());

Review comment:
       Depends whether we want to run TYPE_CHECK. If we want to run type check we have to have these redundant code.




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



[GitHub] [incubator-tvm] antinucleon commented on pull request #6135: [runtime] add float for podvalue

Posted by GitBox <gi...@apache.org>.
antinucleon commented on pull request #6135:
URL: https://github.com/apache/incubator-tvm/pull/6135#issuecomment-663670817


   @tqchen Sounds good.


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



[GitHub] [incubator-tvm] MarisaKirisame commented on a change in pull request #6135: [runtime] add float for podvalue

Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on a change in pull request #6135:
URL: https://github.com/apache/incubator-tvm/pull/6135#discussion_r460087659



##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -371,6 +371,20 @@ class TVMPODValue_ {
     TVM_CHECK_TYPE_CODE(type_code_, kDLFloat);
     return value_.v_float64;
   }
+  operator float() const {
+    double tmp_v = 0;
+    if (type_code_ == kDLInt) {
+      tmp_v = static_cast<double>(value_.v_int64);
+      CHECK_LE(tmp_v, std::numeric_limits<float>::max());

Review comment:
       this three lines are redundent with the below 3 lines. can you have if else that set tmp_v's value?




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



[GitHub] [incubator-tvm] antinucleon closed pull request #6135: [runtime] add float for podvalue

Posted by GitBox <gi...@apache.org>.
antinucleon closed pull request #6135:
URL: https://github.com/apache/incubator-tvm/pull/6135


   


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



[GitHub] [incubator-tvm] MarisaKirisame commented on a change in pull request #6135: [runtime] add float for podvalue

Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on a change in pull request #6135:
URL: https://github.com/apache/incubator-tvm/pull/6135#discussion_r460211589



##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -371,6 +371,20 @@ class TVMPODValue_ {
     TVM_CHECK_TYPE_CODE(type_code_, kDLFloat);
     return value_.v_float64;
   }
+  operator float() const {
+    double tmp_v = 0;
+    if (type_code_ == kDLInt) {
+      tmp_v = static_cast<double>(value_.v_int64);
+      CHECK_LE(tmp_v, std::numeric_limits<float>::max());
+      CHECK_GE(tmp_v, std::numeric_limits<float>::min());
+      return static_cast<float>(tmp_v);
+    }
+    TVM_CHECK_TYPE_CODE(type_code_, kDLFloat);
+    tmp_v = static_cast<double>(value_.v_float64);
+    CHECK_LE(tmp_v, std::numeric_limits<float>::max());
+    CHECK_GE(tmp_v, std::numeric_limits<float>::min());

Review comment:
       ```suggestion
       if (type_code_ == kDLInt) {
         tmp_v = static_cast<double>(value_.v_int64);
       } else {
         TVM_CHECK_TYPE_CODE(type_code_, kDLFloat);
         tmp_v = static_cast<double>(value_.v_float64);
       }
       CHECK_LE(tmp_v, std::numeric_limits<float>::max());
       CHECK_GE(tmp_v, std::numeric_limits<float>::min());
   ```

##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -371,6 +371,20 @@ class TVMPODValue_ {
     TVM_CHECK_TYPE_CODE(type_code_, kDLFloat);
     return value_.v_float64;
   }
+  operator float() const {
+    double tmp_v = 0;
+    if (type_code_ == kDLInt) {
+      tmp_v = static_cast<double>(value_.v_int64);
+      CHECK_LE(tmp_v, std::numeric_limits<float>::max());
+      CHECK_GE(tmp_v, std::numeric_limits<float>::min());
+      return static_cast<float>(tmp_v);
+    }
+    TVM_CHECK_TYPE_CODE(type_code_, kDLFloat);
+    tmp_v = static_cast<double>(value_.v_float64);
+    CHECK_LE(tmp_v, std::numeric_limits<float>::max());
+    CHECK_GE(tmp_v, std::numeric_limits<float>::min());

Review comment:
       Just to be clear I mean like this. I think it is still semantically equivalent.




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