[clr-interp] Fix switch statement stack slot handling #120756
Conversation
…dle dispatch to blocks which already had defined their incoming stack slots
davidwrighton
changed the title
github-actions
bot
added
the
needs-area-label
There was a problem hiding this comment.
Pull Request Overview
Fixes incorrect stack slot handling for switch instructions when multiple case labels branch to the same target basic block. Key changes ensure per-target stack state initialization and variable move emission are performed only once per unique target.
- Added collection of target offsets and deduplication via sorting before emitting per-target setup.
- Moved INTOP_SWITCH instruction emission until after target block preparation.
| lastOffset = targetOffsets[i]; | ||
| InterpBasicBlock *targetBB = m_ppOffsetToBB[targetOffsets[i]]; | ||
| assert(targetBB); | ||
| EmitBBEndVarMoves(targetBB); |
There was a problem hiding this comment.
Doesn't seem like this would work if there are multiple bblocks with already initialized stack state ? What test hits this ? Since we don't even seem to support this scenario on mono, so I'm thinking it is some custom IL testcase ?
There was a problem hiding this comment.
This is hitting in jit/Regression/CLR-x86-JIT/V1-M11-Beta1/b44861/b44861.il I don't see an exclusion for this test in Mono, but shrug. I'll tweak this test to make sure the fix covers multiple bblocks with already initialized stack state, but I think it actually works, since EndBBEndVarMoves doesn't disturb the current stack state. We will end up filling in multiple of the existing stack states with the data before we do the switch, which is pretty terrible codegen, but I don't see why it wouldn't function since our lifetime analysis punts on lifetimes of basic block crossing stack state vars, and promotes them all to global.
There was a problem hiding this comment.
I've added the testcase with multiple sets of pre-existing stack states, and it also works with the fix.
…e-initialized stack state
…dwrighton/runtime into switch_incoming_stack_slots
3 participants
Fix issue where switch instructions did not properly handle dispatch to blocks which already had defined their incoming stack slots