-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Remove fgReplaceSwitchJumpTarget; increase usage of fgReplaceJumpTarget #97664
Remove fgReplaceSwitchJumpTarget; increase usage of fgReplaceJumpTarget #97664
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsIn preparation for improving the proliferation of edge likelihoods (as part of #93020), we should use
|
Diff results for #97664Assembly diffsAssembly diffs for osx/arm64 ran on linux/x64Diffs are based on 2,029,386 contexts (927,368 MinOpts, 1,102,018 FullOpts). MISSED contexts: 109 (0.01%) Overall (-264 bytes)
FullOpts (-264 bytes)
Details here Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64Overall (-0.02% to -0.00%)
MinOpts (-0.08% to +0.00%)
FullOpts (-0.01% to -0.00%)
Throughput diffs for linux/x64 ran on windows/x64Overall (-0.03% to -0.00%)
MinOpts (-0.10% to +0.00%)
FullOpts (-0.03% to -0.00%)
Throughput diffs for osx/arm64 ran on windows/x64Overall (-0.03% to -0.00%)
MinOpts (-0.09% to +0.01%)
FullOpts (-0.01% to -0.00%)
Throughput diffs for windows/arm64 ran on windows/x64Overall (-0.01% to -0.00%)
MinOpts (-0.02% to +0.01%)
FullOpts (-0.01% to -0.00%)
Throughput diffs for windows/x64 ran on windows/x64Overall (-0.01% to -0.00%)
MinOpts (-0.03% to +0.00%)
FullOpts (-0.01% to -0.00%)
Details here |
cc @dotnet/jit-contrib, @AndyAyersMS PTAL. Small diffs due to changes in profile data. |
Diff results for #97664Assembly 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 (-624 bytes)
FullOpts (-624 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 (-904 bytes)
FullOpts (-904 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 (-672 bytes)
FullOpts (-672 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 (-366 bytes)
FullOpts (-366 bytes)
Details here Assembly 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 (+0 bytes)
FullOpts (+0 bytes)
Assembly diffs for windows/x86 ran on windows/x86Diffs are based on 2,290,755 contexts (838,165 MinOpts, 1,452,590 FullOpts). MISSED contexts: 808 (0.04%) Overall (+0 bytes)
FullOpts (+0 bytes)
Details here Throughput diffsThroughput diffs for linux/arm ran on windows/x86Overall (-0.04% to +0.00%)
MinOpts (-0.20% to +0.00%)
Throughput diffs for windows/x86 ran on windows/x86Overall (-0.03% to -0.00%)
MinOpts (-0.02% to +0.00%)
FullOpts (-0.03% to -0.00%)
Details here Throughput diffs for linux/arm64 ran on linux/x64Overall (-0.02% to -0.00%)
MinOpts (-0.08% to +0.00%)
FullOpts (-0.01% to -0.00%)
Throughput diffs for linux/x64 ran on linux/x64Overall (-0.02% to -0.00%)
MinOpts (-0.09% to 0.00%)
FullOpts (-0.02% to -0.00%)
Details 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.
Nit: seems like for these "replace" and "change" APIs with we should be consistent in our ordering of old and new in the arg lists.
My preference would be (block, old, new) but it looks like you are favoring (block, new, old)?
I changed the parameter ordering of |
Diff results for #97664Throughput diffsThroughput diffs for linux/x64 ran on linux/x64Overall (-0.02% to -0.00%)
MinOpts (-0.09% to 0.00%)
FullOpts (-0.02% to -0.00%)
Throughput diffs for osx/arm64 ran on linux/x64Warning: Different compilers used for base and diff JITs. Results may be misleading. Overall (-0.06% to -0.01%)
MinOpts (-0.17% to +0.00%)
FullOpts (-0.03% to -0.02%)
Throughput diffs for windows/arm64 ran on linux/x64Warning: Different compilers used for base and diff JITs. Results may be misleading. Overall (-0.02% to -0.01%)
MinOpts (-0.03% to 0.00%)
FullOpts (-0.03% to -0.02%)
Throughput diffs for windows/x64 ran on linux/x64Warning: Different compilers used for base and diff JITs. Results may be misleading. Overall (-0.01%)
MinOpts (-0.03% to +0.00%)
FullOpts (-0.01%)
Details here |
Failures are known, and last change was just updating a few method signatures. |
Diff results for #97664Assembly diffsAssembly diffs for linux/arm64 ran on windows/x64Diffs are based on 2,507,317 contexts (1,007,092 MinOpts, 1,500,225 FullOpts). MISSED contexts: 1 (0.00%) Overall (-656 bytes)
FullOpts (-656 bytes)
Assembly diffs for linux/x64 ran on windows/x64Diffs are based on 2,517,908 contexts (991,070 MinOpts, 1,526,838 FullOpts). MISSED contexts: 1 (0.00%) Overall (-901 bytes)
FullOpts (-901 bytes)
Assembly diffs for osx/arm64 ran on windows/x64Diffs are based on 2,270,868 contexts (932,669 MinOpts, 1,338,199 FullOpts). MISSED contexts: 2 (0.00%) Overall (-288 bytes)
FullOpts (-288 bytes)
Assembly diffs for windows/arm64 ran on windows/x64Diffs are based on 2,341,108 contexts (938,449 MinOpts, 1,402,659 FullOpts). MISSED contexts: 9 (0.00%) Overall (-664 bytes)
FullOpts (-664 bytes)
Assembly diffs for windows/x64 ran on windows/x64Diffs are based on 2,512,209 contexts (997,391 MinOpts, 1,514,818 FullOpts). MISSED contexts: 3 (0.00%) Overall (-323 bytes)
FullOpts (-323 bytes)
Details here Assembly diffs for linux/arm ran on windows/x86Diffs are based on 2,239,390 contexts (829,328 MinOpts, 1,410,062 FullOpts). MISSED contexts: 71,274 (3.08%) Overall (+0 bytes)
FullOpts (+0 bytes)
Assembly diffs for windows/x86 ran on windows/x86Diffs are based on 2,293,451 contexts (839,658 MinOpts, 1,453,793 FullOpts). MISSED contexts: 45 (0.00%) Overall (+0 bytes)
FullOpts (+0 bytes)
Details here Throughput diffsThroughput diffs for linux/arm64 ran on linux/x64MinOpts (-0.02% to 0.00%)
FullOpts (-0.01% to -0.00%)
Details here Throughput diffs for linux/arm64 ran on windows/x64Warning: Different compilers used for base and diff JITs. Results may be misleading. Overall (-0.02% to -0.01%)
MinOpts (-0.02% to +0.00%)
FullOpts (-0.03% to -0.01%)
Throughput diffs for linux/x64 ran on windows/x64Warning: Different compilers used for base and diff JITs. Results may be misleading. Overall (-0.03% to -0.01%)
MinOpts (-0.10% to -0.00%)
FullOpts (-0.03% to -0.01%)
Details here Throughput diffs for linux/arm ran on windows/x86Warning: Different compilers used for base and diff JITs. Results may be misleading. Overall (-0.03% to -0.00%)
MinOpts (-0.11% to 0.00%)
Throughput diffs for windows/x86 ran on windows/x86Warning: Different compilers used for base and diff JITs. Results may be misleading. Overall (-0.03% to -0.00%)
MinOpts (-0.03% to +0.00%)
FullOpts (-0.03% to -0.00%)
Details here |
Diff results for #97664Assembly diffsAssembly diffs for linux/arm64 ran on windows/x64Diffs are based on 2,507,317 contexts (1,007,092 MinOpts, 1,500,225 FullOpts). MISSED contexts: 1 (0.00%) Overall (-656 bytes)
FullOpts (-656 bytes)
Assembly diffs for linux/x64 ran on windows/x64Diffs are based on 2,517,908 contexts (991,070 MinOpts, 1,526,838 FullOpts). MISSED contexts: 1 (0.00%) Overall (-901 bytes)
FullOpts (-901 bytes)
Assembly diffs for osx/arm64 ran on windows/x64Diffs are based on 2,270,868 contexts (932,669 MinOpts, 1,338,199 FullOpts). MISSED contexts: 2 (0.00%) Overall (-288 bytes)
FullOpts (-288 bytes)
Assembly diffs for windows/arm64 ran on windows/x64Diffs are based on 2,341,108 contexts (938,449 MinOpts, 1,402,659 FullOpts). MISSED contexts: 9 (0.00%) Overall (-664 bytes)
FullOpts (-664 bytes)
Assembly diffs for windows/x64 ran on windows/x64Diffs are based on 2,512,209 contexts (997,391 MinOpts, 1,514,818 FullOpts). MISSED contexts: 3 (0.00%) Overall (-323 bytes)
FullOpts (-323 bytes)
Details here Assembly diffs for linux/arm ran on windows/x86Diffs are based on 2,239,390 contexts (829,328 MinOpts, 1,410,062 FullOpts). MISSED contexts: 71,274 (3.08%) Overall (+0 bytes)
FullOpts (+0 bytes)
Assembly diffs for windows/x86 ran on windows/x86Diffs are based on 2,293,451 contexts (839,658 MinOpts, 1,453,793 FullOpts). MISSED contexts: 45 (0.00%) Overall (+0 bytes)
FullOpts (+0 bytes)
Details here Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64Warning: Different compilers used for base and diff JITs. Results may be misleading. Overall (-0.02% to -0.01%)
MinOpts (-0.02% to +0.00%)
FullOpts (-0.03% to -0.01%)
Details here Throughput diffs for linux/arm ran on windows/x86Warning: Different compilers used for base and diff JITs. Results may be misleading. Overall (-0.03% to -0.00%)
MinOpts (-0.11% to 0.00%)
Throughput diffs for windows/x86 ran on windows/x86Warning: Different compilers used for base and diff JITs. Results may be misleading. Overall (-0.03% to -0.00%)
MinOpts (-0.03% to +0.00%)
FullOpts (-0.03% to -0.00%)
Details here |
In preparation for improving the proliferation of edge likelihoods (as part of #93020), we should use
fgReplaceJumpTarget
where possible, so that we modify predecessor edges in fewer places. I plan on updatingfgReplaceJumpTarget
to always copy the likelihood of the old edge to the new edge as a follow-up to this change. Once I've done that, we can probably usefgReplaceJumpTarget
in more places without losing any edge likelihoods (for example, replacefgTailMergeThrowsJumpToHelper
and its variants with it).