sbinabdullah commented on code in PR #19978:
URL: https://github.com/apache/tvm/pull/19978#discussion_r3558178187
##########
src/arith/const_int_bound.cc:
##########
@@ -291,11 +299,18 @@ class ConstIntBoundAnalyzer::Impl : public
ExprFunctor<ConstIntBoundAnalyzer::En
// Without this optimization: bound = [0, 7167]
// With this optimization: bound = [0, 7152]
if (gcd_coeff_mod > 1) {
- int64_t base_mod = mod_a->base % modulus;
- if (base_mod < 0) base_mod += modulus;
+ int64_t base_mod = mod_a->base % gcd_coeff_mod;
+ if (base_mod < 0) base_mod += gcd_coeff_mod;
int64_t tight_max = modulus - gcd_coeff_mod + base_mod;
- if (tight_max >= modulus) tight_max -= modulus;
- return MakeBound(base_mod, tight_max);
+ if (a.min_value >= 0) {
+ return MakeBound(base_mod, tight_max);
+ }
+ int64_t neg_base = (gcd_coeff_mod - base_mod) % gcd_coeff_mod;
+ int64_t tight_min = -(modulus - gcd_coeff_mod + neg_base);
+ if (a.max_value < 0) {
+ return MakeBound(tight_min, -neg_base);
+ }
+ return MakeBound(tight_min, tight_max);
}
Review Comment:
Good catch — addressed in 6fc6bfc. Both visitors now compute the
interval-based bound first and return `Intersect(interval_bound,
modular_bound)` when the modular fast path applies, so a tight dividend range
is never lost. Added regression cases: `(n*64+63) % 256` with `n in [0,1]` now
gives `[63, 127]` (was `[63, 255]`), plus truncmod/floormod variants with a
negative dividend range.
##########
src/arith/const_int_bound.cc:
##########
@@ -363,10 +383,9 @@ class ConstIntBoundAnalyzer::Impl : public
ExprFunctor<ConstIntBoundAnalyzer::En
// Without this optimization: bound = [0, 7167]
// With this optimization: bound = [0, 7152]
if (gcd_coeff_mod > 1) {
- int64_t base_mod = mod_a->base % modulus;
- if (base_mod < 0) base_mod += modulus;
+ int64_t base_mod = mod_a->base % gcd_coeff_mod;
+ if (base_mod < 0) base_mod += gcd_coeff_mod;
int64_t tight_max = modulus - gcd_coeff_mod + base_mod;
- if (tight_max >= modulus) tight_max -= modulus;
return MakeBound(base_mod, tight_max);
}
Review Comment:
Addressed in 6fc6bfc together with the `Mod` visitor — `FloorMod` now
intersects the modular-set bound with the interval-based bound as well
(regression case: `(n*64+63) % 256` with `n in [0,1]` → `[63, 127]`).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
