tvm-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [incubator-tvm] tqchen commented on a change in pull request #5666: [TIR][Bugfix] Improved massive build times caused by tir.floormod and tir.floordiv. Fixed Topi testcase.
Date Sun, 28 Jun 2020 23:14:33 GMT

tqchen commented on a change in pull request #5666:
URL: https://github.com/apache/incubator-tvm/pull/5666#discussion_r446710438



##########
File path: src/tir/transforms/lower_intrin.cc
##########
@@ -152,14 +172,35 @@ class IntrinInjecter : public tvm::arith::IRMutatorWithAnalyzer {
         }
       }
     } else {
-      // uncommon case
-      DLOG(INFO) << "LowerFloorMod: Cannot decide the sign of divsor and divident";
-      PrimExpr rmod = truncmod(op->a, op->b);
-      // b > 0 && rmod >= 0 -> rmod
-      // b > 0 && rmod < 0  -> rmod + b
-      // b < 0 && rmod < 0 -> rmod
-      // b < 0 && rmod > 0 -> rmod + b
-      return tir::Select((op->b >= 0 && rmod >= 0) || (op->b < 0 &&
rmod <= 0), rmod, rmod + op->b);
+      if (dtype.is_float()) {
+        // a - floor(a / b) * b
+        return op->a - (VisitExpr_(tvm::floor(op->a / op->b).as<CallNode>())
* op->b);
+      } else if (dtype.is_int() && dtype.bits() <= 32) {
+        /* NOTE:
+        This must be restricted to int32 or less since floats can losslessly represent integers
+        only if the number of bits in the mantissa exceeds the number of bits in the integer.
+        Therefore a double (53 bit mantissa) for int32, float (24 bit mantissa) for int16,
etc.
+        Since there is no float128 type, int64 is not supported.
+        */
+
+        // a - floor(a / b) * b
+        auto fdtype = DataType::Float(dtype.bits() * 2, dtype.lanes());
+        auto div = tir::Div(tir::Cast(fdtype, op->a), tir::Cast(fdtype, op->b));

Review comment:
       let us just use the rule in the else branch

##########
File path: src/tir/transforms/lower_intrin.cc
##########
@@ -106,14 +106,34 @@ class IntrinInjecter : public tvm::arith::IRMutatorWithAnalyzer {
         }
       }
     } else {
-      // uncommon case
-      DLOG(INFO) << "LowerFloorDiv: Cannot decide the sign of divisor";
-      // b >= 0 => (rmod >=0 ? rdiv : rdiv - 1)
-      // b < 0  => (rmod <= 0 ? rdiv : rdiv - 1)
-      PrimExpr rdiv = truncdiv(op->a, op->b);
-      PrimExpr rmod = truncmod(op->a, op->b);
-      return tir::Select((op->b >= 0 && rmod >= 0) || (op->b < 0 &&
rmod <= 0), rdiv,
-                         rdiv - make_const(dtype, 1));
+      if (dtype.is_float()) {
+        // floor(a / b)
+        return VisitExpr_(tvm::floor(op->a / op->b).as<CallNode>());
+      } else if (dtype.is_int() && dtype.bits() <= 32) {
+        /* NOTE:
+        This must be restricted to int32 or less since floats can losslessly represent integers
+        only if the number of bits in the mantissa exceeds the number of bits in the integer.
+        Therefore a double (53 bit mantissa) for int32, float (24 bit mantissa) for int16,
etc.
+        Since TVM is unaware of a float128 type, int64 is not supported.
+        */
+
+        // floor(a / b)
+        auto fdtype = DataType::Float(dtype.bits() * 2, dtype.lanes());

Review comment:
       Let us also use the rules in the else branch instead.




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



Mime
View raw message