gemini-code-assist[bot] commented on code in PR #19978:
URL: https://github.com/apache/tvm/pull/19978#discussion_r3558020063
##########
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:
Similar to the `Mod` visitor, immediately returning the modular-set-based
bound for `FloorMod` discards the interval-based bounds of `a`. Intersecting
the modular-set-based bound with the fallback interval-based bound ensures we
do not lose precision when `a` has a tight range.
```c
if (gcd_coeff_mod > 1) {
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;
Entry fallback_bound;
if (a.min_value >= 0) {
if (a.max_value < b.min_value) {
fallback_bound = a;
} else {
fallback_bound = MakeBound(0, std::min(a.max_value,
b_max_cap));
}
} else {
fallback_bound = MakeBound(0, b_max_cap);
}
return Intersect(fallback_bound, MakeBound(base_mod, tight_max));
}
```
##########
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:
When `gcd_coeff_mod > 1`, the modular-set-based bound is immediately
returned, which completely discards the interval-based bounds of the dividend
`a`. If `a` has a tight range (e.g., `a.max_value < b.min_value`), this can
lead to unnecessarily loose bounds. Intersecting the modular-set-based bound
with the fallback interval-based bound preserves maximum precision.
```c
if (gcd_coeff_mod > 1) {
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;
Entry fallback_bound;
if (a.min_value >= 0) {
if (a.max_value < b.min_value) {
fallback_bound = a;
} else {
fallback_bound = MakeBound(0, std::min(a.max_value,
b_max_cap));
}
return Intersect(fallback_bound, 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);
fallback_bound = MakeBound(std::max(a.min_value, -b_max_cap),
std::min(std::max(a.max_value,
(int64_t)0), b_max_cap));
if (a.max_value < 0) {
return Intersect(fallback_bound, MakeBound(tight_min,
-neg_base));
}
return Intersect(fallback_bound, MakeBound(tight_min, tight_max));
}
```
--
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]
