Lunderberg commented on code in PR #13129:
URL: https://github.com/apache/tvm/pull/13129#discussion_r1007156911
##########
src/arith/rewrite_simplify.cc:
##########
@@ -1557,7 +1591,44 @@ PrimExpr RewriteSimplifier::Impl::VisitExpr_(const
NotNode* op) {
}
PrimExpr RewriteSimplifier::Impl::VisitExpr_(const AndNode* op) {
- PrimExpr ret = IRMutatorWithAnalyzer::VisitExpr_(op);
+ PrimExpr ret = [&]() -> PrimExpr {
+ // If this extension isn't enabled, just delegate out.
+ if (!(enabled_extensions_ & kApplyConstraintsToBooleanBranches)) {
+ return IRMutatorWithAnalyzer::VisitExpr_(op);
+ }
+
+ PrimExpr a = op->a;
+ PrimExpr b = op->b;
+
+ // Alternate which branch is used as the constraint, and which
+ // is being simplified. Because some sub-analyzers expect their
+ // constraints to already be simplified, each branch may require
+ // more than update. The loop condition allows each branch to be
+ // visited up to twice, but only if performs the second visit if
+ // necessary.
+ for (size_t i = 0, last_updated = 0; i < last_updated + 2 && i < 4; i++) {
Review Comment:
Yeah, I wasn't sure the best way to express this one. I only wanted to
perform one update per loop iteration, to avoid the fourth simplify (a -> b ->
a -> **b**) if it converged earlier.
Hmm, maybe if I rewrite it as a fixed-length loop with early bail-out
instead. I'd initially written it without the `i < 4`, so that it would
iterate until convergence, but now that I know how the maximum number of loops
required, that flexibility isn't needed anymore.
--
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]
