Allow the simplifier to use facts in its can_prove() predicates. - #9400
Open
mcourteaux wants to merge 9 commits into
Open
Allow the simplifier to use facts in its can_prove() predicates.#9400mcourteaux wants to merge 9 commits into
mcourteaux wants to merge 9 commits into
Conversation
The condition of a can_prove predicate in a rewrite rule was simplified on its own, without any of the facts the simplifier has learned on the way down the IR. Substitute those facts into the condition first, and store facts in the same comparison direction the simplifier produces, so that a fact stated as x > y is usable when it visits y < x. This makes fact-driven rewrite rules possible: max/min now pick a side when the facts order the operands, and a division can cancel a multiplication inside a max or min. Co-authored-by: Claude <noreply@anthropic.com>
Facts and the conditions of can_prove predicates are now looked up in the same canonical form: GT and GE are mapped onto LT, Not is unwrapped, and a comparison can be settled by the other strictness of the same comparison in either direction. This means it no longer matters how a fact was spelled relative to how the rule that consumes it was, and a strict fact such as x > y settles the non-strict predicate the max/min rules ask for. Those rules ask non-strictly, since a tie makes either side of a max or min an equally good answer, so a fact of x >= y is enough to pick a side. Co-authored-by: Claude <noreply@anthropic.com>
Simplifying the condition of a can_prove predicate visits the operands again, so a fact-driven rule that matches every node of its type recursed without bound on nested min/max trees. Disable those rules while inside a can_prove condition; the facts themselves are still substituted in at every level. Co-authored-by: Claude <noreply@anthropic.com>
Recursing further is occasionally useful in principle, but measurably expensive: at a limit of 2, correctness_likely goes from 1.0s to 4.2s and correctness_autodiff from 3.4s to 11.4s, with no test producing a better simplification. Keep the limit at one level, but name the constant. Co-authored-by: Claude <noreply@anthropic.com>
can_prove as a rewrite predicate recursively invokes the simplifier on every expression matching the rule's left-hand side, so a rule whose left-hand side also matches something built while proving the predicate recurses. It is also simply expensive. known_true instead looks the condition up in the facts directly. It cannot recurse, and it is cheap enough to use on a rule that matches every node of its type. The fact-driven max, min and division rules now use it, which is enough for all of them: looking up a comparison already understands direction and strictness. Co-authored-by: Claude <noreply@anthropic.com>
The depth limit was checked in has_facts, which only protects rules that consult it. Checking it on entry to the condition simplification instead protects every can_prove, including the pre-existing rules and any future one, and returning the condition unsimplified is the natural way to decline: the predicate simply fails to prove anything. That also frees has_facts to be a plain check, so the non-recursive known_true rules can fire at any depth. The limit is raised to four, which restricts nothing today: instrumenting every correctness test shows the deepest can_prove nesting any of them reaches is one. Co-authored-by: Claude <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9400 +/- ##
==========================================
- Coverage 70.14% 70.09% -0.05%
==========================================
Files 261 261
Lines 79388 79468 +80
Branches 19357 19381 +24
==========================================
+ Hits 55690 55707 +17
- Misses 17874 17895 +21
- Partials 5824 5866 +42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Refusing to simplify the condition past the depth limit meant the predicate could never be proven there, even when the fact needed was already known. substitute_facts is a plain tree walk (mutate_with over the generic IRMutator base traversal) that never invokes a rewrite rule, so it cannot re-trigger can_prove or known_true and stays safe at any depth: use it as the fallback instead of returning the condition untouched. Added a regression test built on the pre-existing can_prove-based min/max subtraction cancellations in Simplify_Sub.cpp (the rules that motivated the depth limit in the first place, since their predicate constructs a fresh subtraction that can itself match the same rule). With the limit disabled it hangs (confirmed: 15s timeout); with it in place it completes in under a second. Co-authored-by: Claude <noreply@anthropic.com>
The previous fallback ran substitute_facts, a full tree walk, on the condition. But the only thing the caller checks is whether the result is literally the constant true, and nothing runs afterward to fold a compound expression: an And of two individually-known-true operands stays an unfolded And, never becoming true. So substitute_facts's ability to resolve facts about pieces of a compound condition was wasted work here — it can't prove anything is_known_true on the condition itself couldn't already, since folding that partial progress into a verdict is exactly the recursive work the cap exists to avoid. Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem statement
While preparing #9371, I hit several dead ends trying to produce very neat IR. The reason is the simplifier cannot simplify
max(x, y) = xwhen we give the assumptionx >= y. Intuitively, one would write inSimplify_Max.cpp:However, the way
can_prove(Expr, Prover)is implemented is to recursivelymutate()the Expr with the Prover (i.e.,thisinstance ofSimplify). This however, does not substitute in the facts (truths), and therefore fails to "prove" thatx > y.A secondary problem with rewrite rules that use
can_prove()is that they recursively invoke the simplifier, which leads potentially to infinite recursions. Specifically, Andrew stated:Solution
This PR makes it possible to use facts in a
can_prove(), prevents infinite recursion, and offersknown_true()as a lightweight alternative.Using facts / truths.
In order to match simple truths into expressions, whenever new facts are learned, they are canonicalized, such that lookup can do the same, and
a > bmatches withb < a.The
can_prove(Expr, Prover)entry-point now calls out tosimplify_can_prove_condition(), which is the full power of the Simplifier at work, but using canonicalized facts.As a bonus:
substitute_factsbenefits from this canonicalization and manages to substitute in more facts, even when the fact is not IR-tree-wise an exact match.As an additional bonus: weaker forms of inequalities are also substituted in as facts if the stronger inequality is known as a fact. Example:
x >= y(which is weak) is replaced bytrueif the fact thatx > yis known.Limiting recursion.
Implementing this, I indeed hit an infinite recursion quickly, so this PR also limits the recursion depth to 1. Experiments performed with higher recursion depth explode compile time for no measured benefit. The recursion limit of 1 means that a call to
simplify()can callsimplify()only once within acan_prove()but not more. This is done in the callsimplify_can_prove_condition()which is exactly the new behavior ofcan_prove().A lightweight alternative:
known_true().Instead of running the full simplifier for a
can_prove(), we can now also write rewrite-predicates using aknown_true()which is just the simple lookup in the Simplifier's fact list using the canonicalization. This naturally cannot recurse, and also does not spend time trying to simplify things when not needed. This is especially useful for when you would matchmax(x, y)on every such Expr and then try tocan_prove(x > y), which is expensive. Instead the rewrite rule is:Which is a very fast lookup, instead of a whole run through the simplifier.
A few such example rules are implemented in Simplify_Div.
Checklist