JIT: Generalize jump threading very slightly#98107
Merged
jakobbotsch merged 1 commit intodotnet:mainfrom Feb 8, 2024
Merged
Conversation
If the dominator is a direct predecessor then it should be ok to jump thread its direct edge, even if the other edge also reaches.
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsIf the dominator is a direct predecessor then it should be ok to jump thread its direct edge, even if the other edge also reaches. I think.
|
Diff results for #98107Assembly diffsAssembly diffs for linux/x64 ran on windows/x64Diffs are based on 1,621,200 contexts (360,162 MinOpts, 1,261,038 FullOpts). MISSED contexts: base: 2,647 (0.16%), diff: 2,650 (0.16%) Overall (-742 bytes)
FullOpts (-742 bytes)
Assembly diffs for osx/arm64 ran on windows/x64Diffs are based on 1,733,615 contexts (561,303 MinOpts, 1,172,312 FullOpts). MISSED contexts: base: 2,902 (0.17%), diff: 2,906 (0.17%) Overall (-6,596 bytes)
FullOpts (-6,596 bytes)
Assembly diffs for windows/x64 ran on windows/x64Diffs are based on 1,999,660 contexts (587,594 MinOpts, 1,412,066 FullOpts). MISSED contexts: base: 3,225 (0.16%), diff: 3,228 (0.16%) Overall (-4,484 bytes)
FullOpts (-4,484 bytes)
Details here Assembly diffs for linux/arm ran on windows/x86Diffs are based on 1,449,677 contexts (345,734 MinOpts, 1,103,943 FullOpts). MISSED contexts: 55,309 (3.68%) Overall (-2,852 bytes)
FullOpts (-2,852 bytes)
Assembly diffs for windows/x86 ran on windows/x86Diffs are based on 1,625,054 contexts (327,626 MinOpts, 1,297,428 FullOpts). MISSED contexts: base: 4,647 (0.29%), diff: 4,685 (0.29%) Overall (-2,380 bytes)
FullOpts (-2,380 bytes)
Details here Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64Overall (-0.04% to -0.00%)
FullOpts (-0.05% to -0.00%)
Throughput diffs for linux/x64 ran on windows/x64Overall (-0.06% to -0.00%)
FullOpts (-0.07% to -0.00%)
Throughput diffs for osx/arm64 ran on windows/x64Overall (-0.02% to -0.00%)
FullOpts (-0.04% to -0.00%)
Throughput diffs for windows/arm64 ran on windows/x64Overall (-0.05% to -0.00%)
MinOpts (-0.00% to +0.01%)
FullOpts (-0.06% to -0.00%)
Throughput diffs for windows/x64 ran on windows/x64Overall (-0.03% to -0.00%)
FullOpts (-0.03% to -0.00%)
Details here Throughput diffs for linux/arm ran on windows/x86Overall (-0.04% to -0.00%)
FullOpts (-0.04% to -0.00%)
Throughput diffs for windows/x86 ran on windows/x86Overall (-0.05% to -0.00%)
FullOpts (-0.06% to -0.00%)
Details here Throughput diffs for linux/arm64 ran on linux/x64Overall (-0.03% to -0.00%)
FullOpts (-0.03% to -0.00%)
Throughput diffs for linux/x64 ran on linux/x64Overall (-0.05% to -0.00%)
FullOpts (-0.06% to -0.00%)
Details here |
Member
Author
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, Fuzzlyn |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Member
Author
|
cc @dotnet/jit-contrib PTAL @AndyAyersMS. Should be ready, pending CI. Just something minor I noticed while looking at the other example. Diffs are somewhat mixed, though a lot of the benchmarks.run_pgo regressions look to be a repeated one in the same function. |
This was referenced Feb 8, 2024
AndyAyersMS
approved these changes
Feb 8, 2024
Member
AndyAyersMS
left a comment
AndyAyersMS
left a comment
There was a problem hiding this comment.
Thanks for spotting this!
Member
Author
|
Failures look to be known. |
Contributor
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 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.
If the dominator is a direct predecessor then it should be ok to jump thread its direct edge, even if the other edge also reaches. I think.
(Noticed this while looking at the case in #98096 (comment), where we considered
BB33to be an ambiguous pred ofBB38, but it seems we should be able to consider it to be a "true" pred.)