-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
various DCE-related bugfixes #36888
various DCE-related bugfixes #36888
Conversation
Phi nodes are optimized away when there is only one predecessor, but this can cause problems in dead loops because forward references can be created, leading to issues with optimization passes that look at all code, dead or not. This fixes issue JuliaLang#29107 when DCE is turned on.
…ot counted This fixes JuliaLang#29253, which was caused by `simple_dce!` erroneously erasing SSA values that did not appear to be used, because these uses were only discovered in `just_fixup!` at the end of iterating over an `IncrementalCompact`.
PR JuliaLang#36684 changes `iterate(IncrementalCompact)` to return an extra index, but leaves its arguments unchanged. However, the PR decremented the index argument in a particular recursive call to `iterate`. This caused `iterate` not to recognise that it was done when `allow_cfg_transforms` was turned on.
cresult_bbs = let result_bbs = result_bbs, | ||
merged_succ = merged_succ, | ||
merge_into = merge_into, | ||
bbs = bbs, | ||
bb_rename_succ = bb_rename_succ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the let
block prohibits inference of the types of the captured references, which could be a large performance penalty on the inner loops here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I remember adding this let block for that very reason
various DCE-related bugfixes (replaces #36888)
Various DCE-related bugfixes originally from #33730
@Keno @vchuravy