JIT: Remove optLoopCompactionFixupFallThrough #97670
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
ghost
assigned 
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue Details#97488 removed the logic for maintaining implicit fallthrough for BBJ_COND blocks into their false targets via jump insertion, but otherwise left Thanks @jakobbotsch for pointing out we can remove this.
|
2 tasks
Diff results for #97670Assembly diffsAssembly diffs for linux/arm64 ran on windows/x64Diffs are based on 2,259,470 contexts (1,008,044 MinOpts, 1,251,426 FullOpts). MISSED contexts: 159 (0.01%) Overall (-7,024 bytes)
FullOpts (-7,024 bytes)
Assembly diffs for linux/x64 ran on windows/x64Diffs are based on 2,249,703 contexts (981,298 MinOpts, 1,268,405 FullOpts). MISSED contexts: 134 (0.01%) Overall (-15,799 bytes)
FullOpts (-15,799 bytes)
Assembly diffs for osx/arm64 ran on windows/x64Diffs are based on 2,029,386 contexts (927,368 MinOpts, 1,102,018 FullOpts). MISSED contexts: 109 (0.01%) Overall (+1,280 bytes)
FullOpts (+1,280 bytes)
Assembly diffs for windows/arm64 ran on windows/x64Diffs are based on 2,070,850 contexts (937,853 MinOpts, 1,132,997 FullOpts). MISSED contexts: 139 (0.01%) Overall (-4,972 bytes)
FullOpts (-4,972 bytes)
Assembly diffs for windows/x64 ran on windows/x64Diffs are based on 2,098,526 contexts (926,221 MinOpts, 1,172,305 FullOpts). MISSED contexts: 138 (0.01%) Overall (-9,853 bytes)
FullOpts (-9,853 bytes)
Details here Assembly diffs for windows/x86 ran on linux/x86Diffs are based on 2,290,733 contexts (838,165 MinOpts, 1,452,568 FullOpts). MISSED contexts: base: 808 (0.04%), diff: 826 (0.04%) Overall (-13,099 bytes)
FullOpts (-13,099 bytes)
Details here Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64Overall (-0.19% to -0.03%)
FullOpts (-0.21% to -0.03%)
Throughput diffs for linux/x64 ran on windows/x64Overall (-0.17% to -0.03%)
FullOpts (-0.18% to -0.03%)
Throughput diffs for osx/arm64 ran on windows/x64Overall (-0.14% to -0.02%)
FullOpts (-0.17% to -0.03%)
Throughput diffs for windows/arm64 ran on windows/x64Overall (-0.15% to -0.00%)
MinOpts (-0.01% to +0.00%)
FullOpts (-0.18% to -0.00%)
Throughput diffs for windows/x64 ran on windows/x64Overall (-0.17% to -0.00%)
FullOpts (-0.19% to -0.00%)
Details here Throughput diffs for linux/arm64 ran on linux/x64Overall (-0.21% to -0.03%)
FullOpts (-0.24% to -0.03%)
Throughput diffs for linux/x64 ran on linux/x64Overall (-0.20% to -0.03%)
FullOpts (-0.22% to -0.03%)
Details here Throughput diffs for linux/arm ran on windows/x86Overall (-0.12% to -0.03%)
FullOpts (-0.15% to -0.04%)
Throughput diffs for windows/x86 ran on windows/x86Overall (-0.16% to -0.03%)
FullOpts (-0.19% to -0.05%)
Details here |
|
cc @dotnet/jit-contrib, @AndyAyersMS PTAL. Some size improvements/regressions likely stemming from the removed branch reversal. We get a nice TP win from this. |
This was referenced Jan 30, 2024
Diff results for #97670Assembly diffsAssembly diffs for linux/arm ran on windows/x86Diffs are based on 2,053,507 contexts (830,101 MinOpts, 1,223,406 FullOpts). MISSED contexts: 71,368 (3.36%) Overall (-1,482 bytes)
FullOpts (-1,482 bytes)
Details here |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
#97488 removed the logic for maintaining implicit fallthrough for BBJ_COND blocks into their false targets via jump insertion, but otherwise left
optLoopCompactionFixupFallThroughintact. This method also tries to maintain fallthrough behavior by reversing the conditional, if the next block is the true target; per #93020, we should defer condition reversals until block reordering, so we might as well get rid ofoptLoopCompactionFixupFallThrough. By removing this method altogether, we don't have to worry about rebuilding the DFS tree due to the flowgraph being modified, improving TP. This removal caused relatively dramatic diffs locally, which are reduced somewhat by reversing branches (when possible) when restoring implicit fallthrough before block reordering.Thanks @jakobbotsch for pointing out we can remove this.