[Arith] Optional rewriting and simplification into AND of ORs #12972
Conversation
|
This PR is expected to have some merge conflicts with #12863, as they both introduce the same pass context for This is related to the closed #12942. The simplifications in this PR only require a single non-recursive rewrite, but we decided that refactoring this functionality out of |
Lunderberg
force-pushed
the
cnf_simplification
branch
from
51ad989 to
672e670
Compare
Previously, `RewriteSimplifier` inspected the top branch of each `And` and `Or` node, but didn't perform simplifications that would require inspection across branches nested structures. This commit introduces `SimplifyAsAndOfOrs`, which converts to an internal representation as conjunctive normal form, performs simplifications while in that form, then converts back to a `PrimExpr`. This utility is designed for data-propagation simplifications as part of apache#12261. To avoid increasing the CPU load unnecesarily, this utility is only used on an opt-in basis, either using `RewriteSimplifer::SetEnabledFeatures` when called directly, or using `PassContext` when used as part of `tir::transform::Simplify`.
Lunderberg
force-pushed
the
cnf_simplification
branch
from
e1a7c92 to
a90e88e
Compare
|
@tvm-bot rerun |
csullivan
left a comment
csullivan
left a comment
There was a problem hiding this comment.
Looks great! Just a few minor nits and changes requested.
|
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment. Generated by tvm-bot |
csullivan
left a comment
csullivan
left a comment
There was a problem hiding this comment.
LGTM! Thanks @Lunderberg
…#12972) Previously, `RewriteSimplifier` inspected the top branch of each `And` and `Or` node, but didn't perform simplifications that would require inspection across branches nested structures. This commit introduces `SimplifyAsAndOfOrs`, which converts to an internal representation as conjunctive normal form, performs simplifications while in that form, then converts back to a `PrimExpr`. This utility is designed for data-propagation simplifications as part of apache#12261. To avoid increasing the CPU load unnecesarily, this utility is only used on an opt-in basis, either using `RewriteSimplifer::SetEnabledFeatures` when called directly, or using `PassContext` when used as part of `tir::transform::Simplify`. * [Arith][UnitTest] Unittests displaying desired behavior * Corrected example in analyzer.h * Added underscore for private members * Rename Cleanup() to RemoveTrueFalse() * Added comments for conversion to internal representation * Updated comment in test case * Preferentially order clauses according to first occurrence * Added more unit tests
3 participants
Previously,
RewriteSimplifierinspected the top branch of eachAndandOrnode, but didn't perform simplifications that would require inspection across branches nested structures. This commit introducesSimplifyAsAndOfOrs, which converts to an internal representation as conjunctive normal form, performs simplifications while in that form, then converts back to aPrimExpr.This utility is designed for data-propagation simplifications as part of #12261. To avoid increasing the CPU load unnecesarily, this utility is only used on an opt-in basis, either by using RewriteSimplifer::SetEnabledFeatures
when called directly, or by usingPassContextwhen used as part oftir::transform::Simplify`.