gemini-code-assist[bot] commented on code in PR #18699:
URL: https://github.com/apache/tvm/pull/18699#discussion_r2746726772
##########
src/tir/transforms/lower_intrin.cc:
##########
@@ -388,6 +300,49 @@ class IntrinInjecter : public
tvm::arith::IRMutatorWithAnalyzer {
return IRMutatorWithAnalyzer::VisitExpr_(op);
}
+ /*!
+ * \brief Try to find a shift co-efficient c such that a + b*c positive and
does not overflow.
+ *
+ * \param a the dividend
+ * \param b_value the divisor
+ * \return the shift co-efficient c, or nullopt if not found
+ */
+ std::optional<int64_t> TryFindShiftCoefficientForPositiveRange(const
PrimExpr& a,
+ int64_t
b_value) {
+ if (b_value <= 0) {
+ return std::nullopt;
+ }
+ // NOTE: we need to be very careful in the checks below, to make sure
+ // all the intermediate calculations in both compiler checks and runtime
checks
+ // do not overflow
+ arith::ConstIntBound const_int_bound_a = analyzer_->const_int_bound(a);
+ if (const_int_bound_a->min_value >= 0) {
+ return std::nullopt;
+ }
+ const int64_t max_value_of_dtype =
+ Downcast<IntImm>(tvm::max_value(a->dtype.element_of()))->value;
+
+ // NOTE: ensures that (b-1) - a_min does not overflow
+ // also note: max_value_of_dtype + const_int_bound_a->min_value won't
overflow
+ // since a_min is negative, adding it to a positive value will not overflow
+ if (b_value - 1 > max_value_of_dtype + const_int_bound_a->min_value) {
Review Comment:
The overflow check for the intermediate calculation of `c_value` seems
overly conservative. The expression `(b_value - 1) -
const_int_bound_a->min_value` is computed using `int64_t` arithmetic, so the
overflow check should be against `std::numeric_limits<int64_t>::max()`.
However, the current check uses `max_value_of_dtype`, which is the maximum
value of the expression's data type and can be smaller than `int64_t` (e.g.,
for `int32`). This can cause this optimization to be disabled unnecessarily
when it could be safely applied.
Using `ConstIntBoundNode::kPosInf` (which is `LLONG_MAX`) would make this
check more accurate for the `int64_t` calculation, while subsequent checks
correctly validate against `max_value_of_dtype` for parts that are used to
construct TIR expressions of that dtype.
```suggestion
if (b_value - 1 > ConstIntBoundNode::kPosInf +
const_int_bound_a->min_value) {
return std::nullopt;
}
```
--
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]
