yongfeng-nv commented on a change in pull request #5367:
URL: https://github.com/apache/incubator-tvm/pull/5367#discussion_r414263234



##########
File path: src/arith/int_set.cc
##########
@@ -311,6 +311,21 @@ inline IntervalSet Combine<tir::FloorModNode>(Analyzer* 
analyzer,
       LOG(FATAL) << "Modular by zero in CombineInterval Mod";
     }
     if (analyzer->CanProveGreaterEqual(divisor, 0)) {
+      if (const auto* ptr = b->min_value.as<tir::IntImmNode>()) {
+        // a mod b = a - b * (a/b) if
+        // (i) a_max - a_min < b, i.e. that before mod, a's range doesn't 
cover [0, b)
+        // and (ii) a_min mod b <= a_max mod b, i.e. that a's range is still 
continuous after mod
+        auto tmax = a->max_value - b->min_value * floordiv(a->max_value, 
b->min_value);
+        tmax = analyzer->Simplify(tmax);
+        auto tmin = a->min_value - b->min_value * floordiv(a->min_value, 
b->min_value);
+        tmin = analyzer->Simplify(tmin);
+        auto tset = IntervalSet(tmin, tmax);
+        bool within_range = analyzer->CanProveLess(a->max_value - 
a->min_value, ptr->value);
+        bool wrap_around = analyzer->CanProve(tset->max_value < 
tset->min_value);

Review comment:
       I missed the point that CanProve is necessary but not sufficient here.  
Once I modified the condition to tset->max_value >= tset->min_value, 
floormod([z*8+x*4, z*8+x*4+3], 8) fell back to [0, 7], matching your earlier 
suggested floormod implementation, because the it can't prove (((x*4) + 3) - 
(floordiv(((x*4) + 3), 8)*8)) >= ((x*4) - (floordiv(x, 2)*8)).  Therefore, I 
switch to the simple implementation and modify the test.

##########
File path: src/arith/int_set.cc
##########
@@ -311,6 +311,21 @@ inline IntervalSet Combine<tir::FloorModNode>(Analyzer* 
analyzer,
       LOG(FATAL) << "Modular by zero in CombineInterval Mod";
     }
     if (analyzer->CanProveGreaterEqual(divisor, 0)) {
+      if (const auto* ptr = b->min_value.as<tir::IntImmNode>()) {
+        // a mod b = a - b * (a/b) if
+        // (i) a_max - a_min < b, i.e. that before mod, a's range doesn't 
cover [0, b)
+        // and (ii) a_min mod b <= a_max mod b, i.e. that a's range is still 
continuous after mod
+        auto tmax = a->max_value - b->min_value * floordiv(a->max_value, 
b->min_value);
+        tmax = analyzer->Simplify(tmax);
+        auto tmin = a->min_value - b->min_value * floordiv(a->min_value, 
b->min_value);
+        tmin = analyzer->Simplify(tmin);
+        auto tset = IntervalSet(tmin, tmax);
+        bool within_range = analyzer->CanProveLess(a->max_value - 
a->min_value, ptr->value);
+        bool wrap_around = analyzer->CanProve(tset->max_value < 
tset->min_value);
+        if (within_range && !wrap_around) {
+          return tset;

Review comment:
       No longer an issue with the update.




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to