Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Arith][Bugfix] Simplify "x - 1 < y" into "x <= y" #14528

Merged
merged 1 commit into from
Apr 7, 2023

Conversation

Lunderberg
Copy link
Contributor

@Lunderberg Lunderberg commented Apr 7, 2023

This simplification was introduced in #13217, and was erroneously removed in #13933. This commit re-enables this simplification, and adds unit tests to prevent any future regression.

This simplification was introduced in
apache#13217, and was erroneously removed
in apache#13933.  This commit re-enables
this simplification, and adds unit tests to prevent any future
regression.
@tvm-bot
Copy link
Collaborator

tvm-bot commented Apr 7, 2023

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

Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Apr 7, 2023
Previously, so long as `RewriteSimplifier` produces the same output,
unit tests of its behavior would pass.  This could have severe
performance regressions, such as the one resolved in
apache#14528, which caused the runtime of
two test to increase from ~1.5 seconds to ~10 minutes each.

This commit implements statistics counts in RewriteSimplifier, which
are exposed through both the C++ and Python APIs, and uses these to
guard against the known performance regression from
apache#14528.
@areusch areusch merged commit 2eeb37e into apache:main Apr 7, 2023
@Lunderberg Lunderberg deleted the simplify_x_minus_1_lt_y branch April 7, 2023 19:09
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Apr 7, 2023
Previously, so long as `RewriteSimplifier` produces the same output,
unit tests of its behavior would pass.  This could have severe
performance regressions, such as the one resolved in
apache#14528, which caused the runtime of
two test to increase from ~1.5 seconds to ~10 minutes each.

This commit implements statistics counts in RewriteSimplifier, which
are exposed through both the C++ and Python APIs, and uses these to
guard against the known performance regression from
apache#14528.
tqchen pushed a commit that referenced this pull request May 5, 2023
* [Arith] Implement statistics counters for RewriteSimplifier

Previously, so long as `RewriteSimplifier` produces the same output,
unit tests of its behavior would pass.  This could have severe
performance regressions, such as the one resolved in
#14528, which caused the runtime of
two test to increase from ~1.5 seconds to ~10 minutes each.

This commit implements statistics counts in RewriteSimplifier, which
are exposed through both the C++ and Python APIs, and uses these to
guard against the known performance regression from
#14528.

* lint fixes

* Updates based on review comments

* Consistent int64_t with kMaxRecurDepth

* Removed unused is_currently_visiting_

* Add missing \brief for RewriteSimplifierStatsNode

* Use int64_t in ControlFlowGraph for max simplification steps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants