Repository navigation
Compare a kept loop forest with a fresh one by what it says, not by block order - #3708
Merged
Merged
Conversation
…order it lists blocks in Short-circuit keeps the loop forest through each merge. A forest kept that way lists a loop's blocks in the order it found them before the merge, and a fresh build can find the new latch earlier. The -Zverify-each check compared the two with ==, so it called a correct forest stale and the compile failed with E0652, which is how zlib's gzread.c failed at -O2 in the instrumented libraries job. The check now compares the order-free summary the debug assertion in clear_merging already used.
49 tasks done
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.
The "instrumented libraries that have to be right" job on main fails on zlib at
-O2withthe short-circuit pass said it preserved the loop forest of gz_decomp and did not [E0652]. It reproduces on the Linux box withrucc -S -O2 -Zverify-each gzread.c, with or without-fsafety.The forest was right. Short-circuit keeps the loop forest through each merge with
Loops::merged, and a forest kept that way lists a loop's blocks in the order it found them before the merge. A fresh build after the merge found the new latch second rather than last. Printing both forests for gz_decomp showed that one block moving in the list and nothing else. The-Zverify-eachcheck compared the two with==, so it took the order for a difference. The debug assertion inclear_mergingalready compared them withLoops::summary, which sorts every list, and that is what the check now uses too.summaryis no longer debug only.The new test in
analysis.rsis the smallest shape that does it, a loop of three blocks whose latch is folded into the block above it. It fails on the old check and passes now.Checked on the Linux box:
cargo test -p rucc-opt(1,938 unit tests and the integration tests) passes,cargo clippy -p rucc-opt --all-targets -- -D warningsandcargo fmt --all --checkare clean, and every zlib 1.3.1 source compiles with-O2 -Zverify-each.