tqchen commented on code in PR #18330:
URL: https://github.com/apache/tvm/pull/18330#discussion_r2366297107
##########
src/arith/int_set.cc:
##########
@@ -321,6 +323,39 @@ inline IntervalSet Combine<tir::FloorMod>(Analyzer*
analyzer, IntervalSet a, Int
return IntervalSet(tmin, tmax);
}
}
+ // Enhanced: Use ModularSet analysis for better bounds
+ if (auto* div_imm = divisor.as<tir::IntImmNode>()) {
+ int64_t div_val = div_imm->value;
+
+ // Analyze the modular properties of the dividend
+ ModularSet dividend_mod = analyzer->modular_set(op->a);
+
+ if (dividend_mod.defined() && dividend_mod->coeff > 0) {
+ // Calculate GCD of dividend coefficient and divisor
+ int64_t gcd = 1;
+ if (dividend_mod->coeff != 0 && div_val != 0) {
Review Comment:
seems should be lifted into a common function
##########
src/arith/const_int_bound.cc:
##########
@@ -324,6 +343,24 @@ class ConstIntBoundAnalyzer::Impl
if (b.min_value > 0) {
int64_t b_max_cap = InfAwareAdd(b.max_value, -1);
+ // Try to get tighter bounds using modular set information
+ if (parent_ && b.min_value == b.max_value) {
Review Comment:
if we have the bound analysis already in IntervalSet, is the const int bound
still necessary? just want to get a sense of if we need to introduce tihs bound
##########
src/arith/int_set.cc:
##########
@@ -111,8 +112,9 @@ TVM_DECLARE_LOGICAL_OP(Not);
* \brief Combine two interval set under arithmetic operations.
* \note this can possibly relax the set.
*/
-template <typename Op>
-inline IntervalSet Combine(Analyzer* analyzer, IntervalSet a, IntervalSet b,
DataType dtype) {
+template <typename Op, typename OpNode>
+inline IntervalSet Combine(Analyzer* analyzer, IntervalSet a, IntervalSet b,
const OpNode* op) {
Review Comment:
document the op parameter
##########
src/arith/const_int_bound.cc:
##########
@@ -278,6 +278,25 @@ class ConstIntBoundAnalyzer::Impl
if (b.min_value > 0) {
int64_t b_max_cap = InfAwareAdd(b.max_value, -1);
+
+ // 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 = ComputeGCD(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)
Review Comment:
give an example here for readability
##########
src/arith/const_int_bound.cc:
##########
@@ -525,6 +564,22 @@ class ConstIntBoundAnalyzer::Impl
// If the range of b does not have 0, use BinaryOpBoundary.
return BinaryOpBoundary(a, b, op);
}
+ /*!
+ * \brief Compute GCD of two integers.
+ * \param a The first integer.
+ * \param b The second integer.
+ * \return the result.
+ */
+ static int64_t ComputeGCD(int64_t a, int64_t b) {
Review Comment:
https://github.com/apache/tvm/blob/main/src/arith/int_operator.h#L171
--
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]
