JIT: Invalidate m_switchDescMap instead of updating it #98789
Conversation
ghost
assigned
|
/azp run runtime-coreclr superpmi-asmdiffs-checked-release |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr superpmi-asmdiffs-checked-release |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Diff results for #98789Assembly diffsAssembly diffs for linux/arm64 ran on windows/x64Diffs are based on 2,554,585 contexts (1,019,526 MinOpts, 1,535,059 FullOpts). MISSED contexts: 172 (0.01%) Overall (+0 bytes)
FullOpts (+0 bytes)
Assembly diffs for linux/x64 ran on windows/x64Diffs are based on 2,543,224 contexts (988,245 MinOpts, 1,554,979 FullOpts). MISSED contexts: 177 (0.01%) Overall (-3 bytes)
FullOpts (-3 bytes)
Assembly diffs for osx/arm64 ran on windows/x64Diffs are based on 2,317,543 contexts (945,402 MinOpts, 1,372,141 FullOpts). MISSED contexts: 170 (0.01%) Overall (+12 bytes)
FullOpts (+12 bytes)
Assembly diffs for windows/arm64 ran on windows/x64Diffs are based on 2,402,908 contexts (955,693 MinOpts, 1,447,215 FullOpts). MISSED contexts: 174 (0.01%) Overall (+0 bytes)
FullOpts (+0 bytes)
Details here Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64MinOpts (-0.00% to +0.01%)
Throughput diffs for osx/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Details here |
Diff results for #98789Assembly diffsAssembly diffs for osx/arm64 ran on linux/x64Diffs are based on 2,317,543 contexts (945,402 MinOpts, 1,372,141 FullOpts). MISSED contexts: 170 (0.01%) Overall (+12 bytes)
FullOpts (+12 bytes)
Assembly diffs for windows/arm64 ran on linux/x64Diffs are based on 2,402,908 contexts (955,693 MinOpts, 1,447,215 FullOpts). MISSED contexts: 174 (0.01%) Overall (+0 bytes)
FullOpts (+0 bytes)
Details here |
Diff results for #98789Assembly diffsAssembly diffs for linux/arm64 ran on windows/x64Diffs are based on 2,554,585 contexts (1,019,526 MinOpts, 1,535,059 FullOpts). MISSED contexts: 172 (0.01%) Overall (+0 bytes)
FullOpts (+0 bytes)
Assembly diffs for linux/x64 ran on windows/x64Diffs are based on 2,543,224 contexts (988,245 MinOpts, 1,554,979 FullOpts). MISSED contexts: 177 (0.01%) Overall (-3 bytes)
FullOpts (-3 bytes)
Details here |
This reverts commit 77a12ff.
|
/azp run runtime-coreclr superpmi-asmdiffs-checked-release |
|
Azure Pipelines successfully started running 1 pipeline(s). |
amanasifkhalid
changed the title
src/coreclr/jit/fgopt.cpp
Outdated
| if (modified) | ||
| { | ||
| // Invalidate the set of unique targets for block, since we modified the targets | ||
| InvalidateUniqueSwitchSuccMap(); |
There was a problem hiding this comment.
| InvalidateUniqueSwitchSuccMap(); | |
| fgInvalidateSwitchDescMapEntry(block); |
? Or do we need to invalidate the full thing?
There was a problem hiding this comment.
That seems to work; fixed.
|
/azp run runtime-coreclr superpmi-asmdiffs-checked-release |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
cc @dotnet/jit-contrib for visibility. |
amanasifkhalid
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsFixes #98772. Recently, we started checking successor edge likelihoods in This doesn't result in behavioral diffs if the map contains the same state between Debug/Release builds by the time it is read. However, if the map is null by the time it is needed, it is created on-demand, ensuring it is completely up-to-date. If the map is not null, the correctness of its current state depends on how judiciously it was maintained with
|
Diff results for #98789Assembly diffsAssembly diffs for linux/arm64 ran on windows/x64Diffs are based on 2,554,585 contexts (1,019,526 MinOpts, 1,535,059 FullOpts). MISSED contexts: 172 (0.01%) Overall (+0 bytes)
FullOpts (+0 bytes)
Assembly diffs for linux/x64 ran on windows/x64Diffs are based on 2,543,224 contexts (988,245 MinOpts, 1,554,979 FullOpts). MISSED contexts: 177 (0.01%) Overall (-3 bytes)
FullOpts (-3 bytes)
Assembly diffs for osx/arm64 ran on windows/x64Diffs are based on 2,317,543 contexts (945,402 MinOpts, 1,372,141 FullOpts). MISSED contexts: 170 (0.01%) Overall (+12 bytes)
FullOpts (+12 bytes)
Assembly diffs for windows/arm64 ran on windows/x64Diffs are based on 2,402,908 contexts (955,693 MinOpts, 1,447,215 FullOpts). MISSED contexts: 174 (0.01%) Overall (+0 bytes)
FullOpts (+0 bytes)
Details here Throughput diffsThroughput diffs for osx/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Throughput diffs for windows/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Details here |
Diff results for #98789Assembly diffsAssembly diffs for osx/arm64 ran on linux/x64Diffs are based on 2,317,543 contexts (945,402 MinOpts, 1,372,141 FullOpts). MISSED contexts: 170 (0.01%) Overall (+12 bytes)
FullOpts (+12 bytes)
Assembly diffs for windows/arm64 ran on linux/x64Diffs are based on 2,402,908 contexts (955,693 MinOpts, 1,447,215 FullOpts). MISSED contexts: 174 (0.01%) Overall (+0 bytes)
FullOpts (+0 bytes)
Details here Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Details here |
|
|
Diff results for #98789Assembly diffsAssembly diffs for linux/arm64 ran on windows/x64Diffs are based on 2,554,585 contexts (1,019,526 MinOpts, 1,535,059 FullOpts). MISSED contexts: 172 (0.01%) Overall (+0 bytes)
FullOpts (+0 bytes)
Assembly diffs for linux/x64 ran on windows/x64Diffs are based on 2,543,224 contexts (988,245 MinOpts, 1,554,979 FullOpts). MISSED contexts: 177 (0.01%) Overall (-3 bytes)
FullOpts (-3 bytes)
Assembly diffs for osx/arm64 ran on windows/x64Diffs are based on 2,317,543 contexts (945,402 MinOpts, 1,372,141 FullOpts). MISSED contexts: 170 (0.01%) Overall (+12 bytes)
FullOpts (+12 bytes)
Assembly diffs for windows/arm64 ran on windows/x64Diffs are based on 2,402,908 contexts (955,693 MinOpts, 1,447,215 FullOpts). MISSED contexts: 174 (0.01%) Overall (+0 bytes)
FullOpts (+0 bytes)
Details here |
3 participants
Fixes #98772. Recently, we started checking successor edge likelihoods in
Compiler::fgDebugCheckProfileWeightsby default; this means we callCompiler::fgDebugCheckOutgoingProfileDatain Debug/Checked builds to verify successor edges' likelihoods, and thus callBasicBlock::GetSucc(unsigned, Compiler*)to iterate the successor edges. For switch blocks,GetSucc(unsigned, Compiler*)callsGetSwitchDescMap, and buildsm_switchDescMapif it doesn't exist yet. Upon finishing edge likelihood verification, we don't resetm_switchDescMap, so it is possible for this map to be created earlier in Debug/Checked builds versus Release builds.This doesn't result in behavioral diffs if the map contains the same state between Debug/Release builds by the time it is read. However, if the map is null by the time it is needed, it is created on-demand, ensuring it is completely up-to-date. If the map is not null, the correctness of its current state depends on how judiciously it was maintained with
UpdateSwitchTableTarget, creating potential for diffs in the map's state. By invalidating the map instead of updating it as state changes, we can force it to be rebuilt with the latest state when it's needed.