Cranelift: remove slow invariant validation in cfg(fuzzing) from MachBuffer. - #4038
Merged
alexcrichton merged 1 commit intoApr 15, 2022
Conversation
…Buffer. Following the merge of regalloc2 support, this became slower because we are stricter about the critical-edge invariant, generating a separate edge block for every out-edge even if two or more out-edges go to the same successor (this is significant in cases of `br_table` with many entries having the same target block, for example). Many of those edge blocks are empty and end up collapsed by the MachBuffer, which leads to a large set of aliased labels. The invariant validation will dutifully iterate over all the data structures at every step, validating all of our conditions. But this gets way slower in the new context, to the point that we'll probably have some fuzz timeouts. This was pointed out in [1] but I missed removing this in bytecodealliance#3989. Given that `MachBuffer` has been around for nearly two years now, has been fuzzed continuously with the invariant validation for that time, and also has a correctness proof in the comments, it's probably reasonable to remove this high (recently increased) cost from the fuzzing-specific compilation configuration. [1] bytecodealliance#3989 (comment)
github-actions
Bot
added
cranelift
alexcrichton
approved these changes
Apr 15, 2022
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
2 participants
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.
Following the merge of regalloc2 support, this became slower because we
are stricter about the critical-edge invariant, generating a separate
edge block for every out-edge even if two or more out-edges go to the
same successor (this is significant in cases of
br_tablewith manyentries having the same target block, for example).
Many of those edge blocks are empty and end up collapsed by the
MachBuffer, which leads to a large set of aliased labels.
The invariant validation will dutifully iterate over all the data
structures at every step, validating all of our conditions. But this
gets way slower in the new context, to the point that we'll probably
have some fuzz timeouts.
This was pointed out in [1] but I missed removing this in #3989. Given
that
MachBufferhas been around for nearly two years now, has beenfuzzed continuously with the invariant validation for that time, and
also has a correctness proof in the comments, it's probably reasonable
to remove this high (recently increased) cost from the fuzzing-specific
compilation configuration.
[1]
#3989 (comment)