-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[Arith] Fix const-int-bound modular-set tightening for Mod/FloorMod #19978
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ed75bb9
f068050
eccd4ae
cf7b5c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -273,15 +273,52 @@ class ConstIntBoundAnalyzer::Impl : public ExprFunctor<ConstIntBoundAnalyzer::En | |
| if (b.min_value > 0) { | ||
| int64_t b_max_cap = InfAwareAdd(b.max_value, -1); | ||
|
|
||
| // Interval-based bound of the truncated mod. | ||
| Entry interval_bound; | ||
| if (a.min_value >= 0) { | ||
| // 0 <= [a_min, a_max] < b_min | ||
| if (a.max_value < b.min_value) { | ||
| interval_bound = a; | ||
| } else { | ||
| // other case, we can get close to 0 | ||
| interval_bound = MakeBound(0, std::min(a.max_value, b_max_cap)); | ||
| } | ||
| } else if (a.max_value < 0) { | ||
| // The dividend is entirely negative. The truncated result keeps the | ||
| // sign of the dividend, so it is in [-(b-1), 0]. If additionally | ||
| // |a| < b for every value in range (a.min_value > -b.min_value), | ||
| // no reduction happens and the result equals a, giving the tight | ||
| // [a.min, a.max]. This mirrors the non-negative "return a" case | ||
| // above; without it the upper bound would be the loose 0. | ||
| if (a.min_value > -b.min_value) { | ||
| interval_bound = a; | ||
| } else { | ||
| interval_bound = MakeBound(std::max(a.min_value, -b_max_cap), 0); | ||
| } | ||
| } else { | ||
| interval_bound = MakeBound(std::max(a.min_value, -b_max_cap), | ||
| std::min(std::max(a.max_value, (int64_t)0), b_max_cap)); | ||
| } | ||
|
|
||
| // Try to get tighter bounds using modular set information | ||
| if (parent_ && b.min_value == b.max_value) { | ||
| ModularSet mod_a = parent_->modular_set(op->a); | ||
| int64_t modulus = b.min_value; | ||
| int64_t gcd_coeff_mod = ZeroAwareGCD(mod_a->coeff, modulus); | ||
|
|
||
| // If gcd_coeff_mod > 1, we can get tighter bounds | ||
| // The result will be of the form gcd_coeff_mod * k + (base % modulus) | ||
| // where k ranges to cover [0, modulus - gcd_coeff_mod] | ||
| // If gcd_coeff_mod > 1, we can get tighter bounds. | ||
| // Since gcd_coeff_mod divides both mod_a->coeff and modulus, we know | ||
| // a == mod_a->base (mod gcd_coeff_mod). Truncated mod keeps that | ||
| // residue on the non-negative side and mirrors it on the negative | ||
| // side, so with base_mod = mod_a->base % gcd_coeff_mod (normalized | ||
| // to [0, gcd_coeff_mod)): | ||
| // non-negative results are in {base_mod, base_mod + gcd, ..., | ||
| // modulus - gcd + base_mod} | ||
| // negative results (only if a can be negative) are the mirrored | ||
| // set {-(modulus - gcd + neg_base), ..., -neg_base} with | ||
| // neg_base = (gcd - base_mod) % gcd. | ||
| // The modular bound is intersected with the interval bound so a | ||
| // tight dividend range is never lost. | ||
| // | ||
| // Example: expr = (bx * 2048 + tx * 16) % 7168 | ||
| // where bx in [0, 3584), tx in [0, 128) | ||
|
|
@@ -291,23 +328,26 @@ 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); | ||
| Entry modular_bound; | ||
| if (a.min_value >= 0) { | ||
| modular_bound = MakeBound(base_mod, tight_max); | ||
| } else { | ||
| 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) { | ||
| modular_bound = MakeBound(tight_min, -neg_base); | ||
| } else { | ||
| modular_bound = MakeBound(tight_min, tight_max); | ||
| } | ||
| } | ||
| return Intersect(interval_bound, modular_bound); | ||
| } | ||
|
Comment on lines
330
to
347
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When 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));
}
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. addressed in 6fc6bfc. Both visitors now compute the interval-based bound first and return |
||
| } | ||
|
|
||
| if (a.min_value >= 0) { | ||
| // 0 <= [a_min, a_max] < b_min | ||
| if (a.max_value < b.min_value) return a; | ||
| // other case, we can get close to 0 | ||
| return MakeBound(0, std::min(a.max_value, b_max_cap)); | ||
| } else { | ||
| return MakeBound(std::max(a.min_value, -b_max_cap), | ||
| std::min(std::max(a.max_value, (int64_t)0), b_max_cap)); | ||
| } | ||
| return interval_bound; | ||
| } else { | ||
| TVM_FFI_ICHECK(!b.is_const(0)) << "mod by zero"; | ||
| // mod by negative value is rare, | ||
|
|
@@ -345,15 +385,38 @@ class ConstIntBoundAnalyzer::Impl : public ExprFunctor<ConstIntBoundAnalyzer::En | |
|
|
||
| if (b.min_value > 0) { | ||
| int64_t b_max_cap = InfAwareAdd(b.max_value, -1); | ||
|
|
||
| // Interval-based bound of the floor mod (result is always in | ||
| // [0, b_max_cap] for a positive divisor). | ||
| Entry interval_bound; | ||
| if (a.min_value >= 0) { | ||
| // 0 <= [a_min, a_max] < b_min | ||
| if (a.max_value < b.min_value) { | ||
| interval_bound = a; | ||
| } else { | ||
| // other case, we can get close to 0 | ||
| interval_bound = MakeBound(0, std::min(a.max_value, b_max_cap)); | ||
| } | ||
| } else { | ||
| interval_bound = MakeBound(0, b_max_cap); | ||
| } | ||
|
|
||
| // Try to get tighter bounds using modular set information | ||
| if (parent_ && b.min_value == b.max_value) { | ||
| ModularSet mod_a = parent_->modular_set(op->a); | ||
| int64_t modulus = b.min_value; | ||
| int64_t gcd_coeff_mod = ZeroAwareGCD(mod_a->coeff, modulus); | ||
|
|
||
| // If gcd_coeff_mod > 1, we can get tighter bounds | ||
| // The result will be of the form gcd_coeff_mod * k + (base % modulus) | ||
| // where k ranges to cover [0, modulus - gcd_coeff_mod] | ||
| // If gcd_coeff_mod > 1, we can get tighter bounds. | ||
| // Since gcd_coeff_mod divides both mod_a->coeff and modulus, we know | ||
| // a == mod_a->base (mod gcd_coeff_mod), and therefore | ||
| // floormod(a, modulus) == base_mod (mod gcd_coeff_mod), where | ||
| // base_mod = mod_a->base % gcd_coeff_mod (normalized to | ||
| // [0, gcd_coeff_mod)). The result (always in [0, modulus)) is thus | ||
| // in {base_mod, base_mod + gcd_coeff_mod, ..., | ||
| // modulus - gcd_coeff_mod + base_mod}. | ||
| // The modular bound is intersected with the interval bound so a | ||
| // tight dividend range is never lost. | ||
| // | ||
| // Example: expr = (bx * 2048 + tx * 16) % 7168 | ||
| // where bx in [0, 3584), tx in [0, 128) | ||
|
|
@@ -363,22 +426,14 @@ 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); | ||
| return Intersect(interval_bound, MakeBound(base_mod, tight_max)); | ||
| } | ||
|
Comment on lines
428
to
433
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the 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));
}
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 6fc6bfc together with the |
||
| } | ||
|
|
||
| if (a.min_value >= 0) { | ||
| // 0 <= [a_min, a_max] < b_min | ||
| if (a.max_value < b.min_value) return a; | ||
| // other case, we can get close to 0 | ||
| return MakeBound(0, std::min(a.max_value, b_max_cap)); | ||
| } else { | ||
| return MakeBound(0, b_max_cap); | ||
| } | ||
| return interval_bound; | ||
| } else { | ||
| TVM_FFI_ICHECK(!b.is_const(0)) << "floormod by zero"; | ||
| int64_t b_min_cap = InfAwareAdd(b.min_value, 1); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe there can be tigher bound when amax < 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really good catch I think you’re right. will add an additional check for this.