gemini-code-assist[bot] commented on code in PR #18694:
URL: https://github.com/apache/tvm/pull/18694#discussion_r2740450895
##########
src/tir/transforms/lower_intrin.cc:
##########
@@ -214,10 +215,11 @@ class IntrinInjecter : public
tvm::arith::IRMutatorWithAnalyzer {
// If the numerator's lower bound is known, express the floormod
// in terms of truncmod using only positive operands.
- arith::ConstIntBound const_int_bound = analyzer_->const_int_bound(op->a);
- if (const_int_bound->min_value < 0 &&
- const_int_bound->min_value >
-
-(Downcast<IntImm>(tvm::max_value(op->a->dtype.element_of()))->value)) {
+ arith::ConstIntBound const_int_bound_a =
analyzer_->const_int_bound(op->a);
+ arith::ConstIntBound const_int_bound_b =
analyzer_->const_int_bound(op->b);
+ if (const_int_bound_a->min_value < 0 &&
+ const_int_bound_b->max_value - const_int_bound_a->min_value <
+
(Downcast<IntImm>(tvm::max_value(op->a->dtype.element_of()))->value) + 2) {
Review Comment:
Similar to the `FloorDiv` case, extracting the maximum value of the type
into a local variable here would improve readability and avoid re-computation.
```suggestion
arith::ConstIntBound const_int_bound_a =
analyzer_->const_int_bound(op->a);
arith::ConstIntBound const_int_bound_b =
analyzer_->const_int_bound(op->b);
const int64_t max_value_of_type =
Downcast<IntImm>(tvm::max_value(op->a->dtype.element_of()))->value;
if (const_int_bound_a->min_value < 0 &&
const_int_bound_b->max_value - const_int_bound_a->min_value <
max_value_of_type + 2) {
```
##########
src/tir/transforms/lower_intrin.cc:
##########
@@ -118,10 +118,11 @@ class IntrinInjecter : public
tvm::arith::IRMutatorWithAnalyzer {
// If the numerator's lower bound is known, express the floordiv
// in terms of truncdiv using only positive operands.
- arith::ConstIntBound const_int_bound = analyzer_->const_int_bound(op->a);
- if (const_int_bound->min_value < 0 &&
- const_int_bound->min_value >
-
-(Downcast<IntImm>(tvm::max_value(op->a->dtype.element_of()))->value)) {
+ arith::ConstIntBound const_int_bound_a =
analyzer_->const_int_bound(op->a);
+ arith::ConstIntBound const_int_bound_b =
analyzer_->const_int_bound(op->b);
+ if (const_int_bound_a->min_value < 0 &&
+ const_int_bound_b->max_value - const_int_bound_a->min_value <
+
(Downcast<IntImm>(tvm::max_value(op->a->dtype.element_of()))->value) + 2) {
Review Comment:
For improved readability and to avoid repeating a complex expression,
consider extracting the maximum value of the type into a local `const` variable
before the `if` statement.
```suggestion
arith::ConstIntBound const_int_bound_a =
analyzer_->const_int_bound(op->a);
arith::ConstIntBound const_int_bound_b =
analyzer_->const_int_bound(op->b);
const int64_t max_value_of_type =
Downcast<IntImm>(tvm::max_value(op->a->dtype.element_of()))->value;
if (const_int_bound_a->min_value < 0 &&
const_int_bound_b->max_value - const_int_bound_a->min_value <
max_value_of_type + 2) {
```
--
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]
