Fix CFG corruption in CFG simplify #48962
Merged
Conversation
This file contains 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
IncrementalCompact ordinarily takes ownership of the CFG in order to to its transform. cfg_simplify! separate constructs the CFG transform structures ahead of time and was assuming this meant that the original CFG remained untouched (since it was using it for lookup operations). Unfortunately, the IncrementalCompact constructor was already doing some CFG manipulation cuasing the CFG to be corrupted and cfg_simplify! to create invalid IR. Fix that by refactoring the IncrementalCompact constructor to allow passing in the CFG transformation state explicitly, rather than poking it into the fields afterwards.
vtjnash
approved these changes
Mar 10, 2023
aviatesk
added a commit
that referenced
this pull request
Mar 17, 2023
`bb_rename_pred` was captured, so this commit fixes it up.
FWIW, `JET.report_opt(CC.batch_inline!,
(CC.IRCode,Vector{Pair{Int,Any}},Bool,CC.OptimizationParams))`
successfully detected it.
vtjnash
pushed a commit
that referenced
this pull request
Mar 17, 2023
oscardssmith
pushed a commit
to oscardssmith/julia
that referenced
this pull request
Mar 20, 2023
…ang#49036) `bb_rename_pred` was captured by JuliaLang#48962, so this commit fixes it up. This was successfully detected by: `JET.report_opt(CC.batch_inline!, (CC.IRCode,Vector{Pair{Int,Any}},Bool,CC.OptimizationParams))`
Xnartharax
pushed a commit
to Xnartharax/julia
that referenced
this pull request
Apr 19, 2023
IncrementalCompact ordinarily takes ownership of the CFG in order to to its transform. cfg_simplify! separate constructs the CFG transform structures ahead of time and was assuming this meant that the original CFG remained untouched (since it was using it for lookup operations). Unfortunately, the IncrementalCompact constructor was already doing some CFG manipulation cuasing the CFG to be corrupted and cfg_simplify! to create invalid IR. Fix that by refactoring the IncrementalCompact constructor to allow passing in the CFG transformation state explicitly, rather than poking it into the fields afterwards.
Xnartharax
pushed a commit
to Xnartharax/julia
that referenced
this pull request
Apr 19, 2023
…ang#49036) `bb_rename_pred` was captured by JuliaLang#48962, so this commit fixes it up. This was successfully detected by: `JET.report_opt(CC.batch_inline!, (CC.IRCode,Vector{Pair{Int,Any}},Bool,CC.OptimizationParams))`
oscardssmith
added
bugfix
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.
IncrementalCompact ordinarily takes ownership of the CFG in order to to its transform. cfg_simplify! separate constructs the CFG transform structures ahead of time and was assuming this meant that the original CFG remained untouched (since it was using it for lookup operations). Unfortunately, the IncrementalCompact constructor was already doing some CFG manipulation cuasing the CFG to be corrupted and cfg_simplify! to create invalid IR. Fix that by refactoring the IncrementalCompact constructor to allow passing in the CFG transformation state explicitly, rather than poking it into the fields afterwards.