Lunderberg commented on code in PR #11646:
URL: https://github.com/apache/tvm/pull/11646#discussion_r894624013
##########
src/arith/const_int_bound.cc:
##########
@@ -341,6 +343,18 @@ class ConstIntBoundAnalyzer::Impl
}
}
+ Entry VisitLeftShift(const CallNode* op) {
+ Entry a = VisitExpr(op->args[0]);
+ Entry b = VisitExpr(op->args[1]);
+
+ // Until C++20, performing a left shift is only well-defined for
+ // positive arguments. If we have a negative argument, it just
+ // means we couldn't prove that the inputs were positive.
+ a.min_value = std::max(int64_t(0), a.min_value);
+ b.min_value = std::max(int64_t(0), b.min_value);
+ return BinaryOpBoundary(a, b, InfAwareLeftShift);
Review Comment:
Hmm, this might depend on the target. For the `CodegenC` backend, it
doesn't narrow the bounds for any legal use of `<<`, because negative arguments
are entirely undefined behavior anyways. Taking the `max` here is a way to
express that same constraint. That is, even if we can't prove that the
argument is non-negative, its use in a bitshifting operator provides a
constraint.
That said, since my primary goal is to improve simplifications from
`ceil_log2`, perhaps it would be better to look for a `ceil(log2(x))` call, and
use that directly. The aggressive optimizations that C++ compilers make based
on undefined behavior reasoning are controversial for a reason, and I'd like to
avoid introducing similar logic in TVM unless required.
--
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]
