Lunderberg commented on code in PR #13130:
URL: https://github.com/apache/tvm/pull/13130#discussion_r1023056526
##########
src/arith/conjunctive_normal_form.cc:
##########
@@ -248,14 +248,14 @@ void AndOfOrs::TrySimplifyOr(Key* a_ptr, Key* b_ptr,
Analyzer* analyzer) {
Key& a = *a_ptr;
Key& b = *b_ptr;
PrimExpr joint = GetExpr(a) || GetExpr(b);
- PrimExpr simplified = analyzer->Simplify(joint);
+ PrimExpr simplified = analyzer->rewrite_simplify(joint);
if (!ExprDeepEqual()(simplified, joint)) {
if (auto* simplified_or = simplified.as<OrNode>()) {
a = GetKey(simplified_or->a);
b = GetKey(simplified_or->b);
} else {
- a = GetKey(simplified);
- b = key_false_;
+ a = key_false_;
+ b = GetKey(simplified);
Review Comment:
Not so much as bug as a performance improvement and consistency with some
steps in the data-flow portions.
The performance improvement comes from [this calling
scope](https://github.com/apache/tvm/blob/main/src/arith/conjunctive_normal_form.cc#L286),
which iterates across pairs of expressions, where `b` is always later in the
loop. By placing the simplified expression in `b` instead of `a`, this allows
later calls to `TrySimplifyOr` to further simplify using the simplified
expression as an input. As a result, repeated passes across all pairs are
avoided.
The consistency comes from the `ControlFlowBlock`'s use of `ExprDeepEqual`
to check for convergence. Because the rewrite simplifier doesn't produce a
canonical form (e.g. The expressions `i > 0 && j > 0` and `j>0 && i>0` are
equivalent, but neither will be simplified to the other), there were a few
cases where I wanted to preserve a specific order when simplifying.
--
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]
