Lunderberg commented on code in PR #11646:
URL: https://github.com/apache/tvm/pull/11646#discussion_r902693965
##########
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:
Good point. I had assumed that this was safe, because a similar
compile-time check is used for the constant folding in `operator<<`, but in
that case the left shift would be entirely removed. By removing a conditional,
but still allowing the left shift to remain, we're asserting that we are
handling the left shift identically to how it will be at runtime.
I've updated the handling of left shift to return `Everything` in the case
of potentially negative arguments, and added specific handling for
`ceil(log2(x))`.
--
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]
