Repository navigation
C3: validation visits each shared term once - #101
Merged
Merged
Conversation
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.
#64 made the goals of a module share the terms of their assumptions, but the traversals that check a module still walked every goal as a tree. Each obligation's goal states every assumption before it, so a function's checks revisited its first assumptions once per later check, quadratic in its length.
validateObligationModuleruns on every module that is built, simplified, or replayed, and it was 80% of lowering. callgrind on--lower-only --jobs=1 suite/safe_fib.cppcounts 20.4 of 25.1 billion instructions under it, mostly in:validateLogicExprvalidateVariableScopes, which also copies its set of bound names at every nodestring_view::find, its embedded-NUL checkscollectRequiredFeaturesvalidateLogicCallscollectQuantifierBindersThis is step 2 of plan item C3 (gap G4): memoized traversals. Step 1 was #64.
Results
--lower-only --jobs=1, release build, wall-clock time and peak RSS,main(0290cc7) against this branch:mainsuite/safe_fib.cppsuite/llvm_uleb128.cppintterms (gen_deep.py 250)This meets the plan's targets for
safe_fib.cpp(under 3 s) and the ULEB128 case study (under 6 s). After this PR, the run takes 6.2 billion instructions in all, of which validation is 1.8 billion (29%). The rest is mostly Z3 lowering of every query under--lower-only(34%) and passivization (14%).Long sums (F27) barely change: their cost is the VCR copies of each overflow check's operands, not Layer 3. They are left to the later steps of C3.
With
--jobs=8,--lower-onlyonsuite/llvm_uleb128_decoder_exhaustive.cpp(UNSUPPORTED in lit) takes 23 s within 7.8 GB, against 319 s within 7.7 GB onmain; the sets of checked terms take the 0.1 GB more.Design
Checks that depend on the term alone visit each term once per module. These are the form of each node, that every application has a declaration with its signature, and the logic features the module needs.
validateLogicExprandvalidateLogicCallskeep the terms that passed in a set, andcollectRequiredFeaturesthe terms whose features it has already collected. A term in the set is not checked again, whichever root reaches it.Scope depends on where a term is, so
ScopeValidatorchecks it in two parts:std::set<std::string>.BinderSummariescomputes two interned sets once for each term: the binder names it contains, and the binder names free in it. Free names are computed only after every binder name of the module is registered.bindandvarsinsrc/lib/qed/term.ml), and how Lean caches eachExpr's loose bound-variable range.The reference check decides every failure. A root that fails either part is checked again by the tree walk that was the whole check before. So every error message, and which error comes first, is what it was. An assertion checks that this walk fails too: the summaries never reject what the walk accepts.
Node counts.
countModuleNodes, the simplifier's statistics, counts a module's nodes as trees, as before. It now computes each shared term's count once, so the numbers are the same in linear time.Hashing and serialization still walk trees. They run with
--obligation-out, the proof cache, and Layer 3 dumps.Tests
unittests/Verify/ObligationValidationTest.cpp(new) builds modules whose roots share terms, which archive replay cannot do, since a record does not share nodes. It covers:main's validator walks 2^26 paths and did not finish this test with 28 levels in 600 s.Every test but SharedTermsAreCheckedOnce passes against
main's validator too, with the same messages.Each mutant below, applied alone, fails a test:
obligation_archive.testobligation_archive.testobligation_archive.testobligation_archive.test,obligation_ir.cppV4 and W4 survived the first sweep. They only make validation slower, since the reference walk then decides. The sharing test and the assertion were added for them.
Docs
Checked
--lower-only --jobs=1 --dump-ir=3, semantic hashes included) of all 431.cppsources underclang/test/Verify,mainagainst this branch, are byte-identical, except for:crash_report_url.cpp, which crashes on purpose: only the binary's name and addresses in the stack trace differ;suite/llvm_uleb128_decoder_exhaustive.cpp, whose dumps are too large to compare. Its archives (--obligation-out, 238 MB) are byte-identical instead, with the same records refused for the archive's expression limit on both.clang/test/Verifyon this branch: 473 passed, 1 unsupported (darwin only), no failure.VerifyTestson this branch: 69 passed, and the opt-in corpus parity check skipped (it needsCPPVERIFY_PARITY_ARCHIVES).