JIT: fix bit-test switch lowering when the bit table is inverted - #131736
Merged
Conversation
TryLowerSwitchToBitTest inverts the bit table on xarch when its upper 32 bits are all set, so that it still fits in a 32 bit immediate, and swaps the two targets. The swap only updated bbCase0/bbCase1, but the code that follows consumes case0Edge/case1Edge, so the JCC was wired to the wrong successor and the block ref counts were decremented on the wrong blocks. Swap the edges instead and derive the blocks afterwards. The inversion can only trigger with exactly 64 bit table entries: below that, the bits above bitCount are zero, so ~bitTable exceeds UINT32_MAX. Fixes dotnet#131716 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c492a8b-6607-48dc-8a2e-98128e3197b0
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a correctness issue in CoreCLR JIT switch-to-bit-test lowering on xarch when the bit table is inverted to fit a 32-bit immediate, ensuring the conditional branch and ref-count adjustments use the correct successor edges after inversion.
Changes:
- Update
TryLowerSwitchToBitTestto swapcase0Edge/case1Edge(not just destination blocks) when inverting the bit table, and derivebbCase0/bbCase1after the swap. - Add a JIT regression test covering the “exactly 64 cases + inverted bit table” scenario and a non-inverted control case.
- Include the new regression test in the
Regression_ro_2test project.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/jit/lower.cpp | Fixes edge swapping during inverted bit-table lowering so SetCond and dup-count fixups target the correct successors. |
| src/tests/JIT/Regression/JitBlue/Runtime_131716/Runtime_131716.cs | Adds a regression test that reproduces the inverted-bit-table 64-case switch shape and validates correct dispatch. |
| src/tests/JIT/Regression/Regression_ro_2.csproj | Adds the new regression test source file to the project build. |
…vert # Conflicts: # src/tests/JIT/Regression/Regression_ro_2.csproj
jakobbotsch
approved these changes
Aug 3, 2026
4 tasks
JulieLeeMSFT
pushed a commit
that referenced
this pull request
Aug 4, 2026
…s inverted (#131781) Backport of #131736 to release/10.0 ## Customer Impact - [x] Customer reported - [ ] Found internally Reported in #131716. Wrong code on x64: a `switch` with exactly 64 cases and two distinct targets branches to the *opposite* target. `TryLowerSwitchToBitTest` inverts the bit table when its upper 32 bits are all set (so the table still fits a 32-bit immediate) and swaps the two targets, but the swap only updated `bbCase0`/`bbCase1` while the code after it consumes `case0Edge`/`case1Edge`. So `SetCond` wired the `JCC` to the wrong successor and the dup-count fixup decremented `bbRefs` on the wrong blocks. This shape is very easy to hit from Roslyn's `async` state-machine dispatch: an `async IAsyncEnumerable<T>`/`async` method with 61 awaits+yields produces exactly a 64-entry, two-target jump table. The reporter saw the iterator body re-entered millions of times (hang) and frameless `NullReferenceException`s on captured variables. Reproduces at Tier0, MinOpts and fully optimized code. ## Regression - [x] Yes - [ ] No Introduced in .NET 10 by #116933, which replaced the `fgRemoveAllRefPreds`/`fgAddRefPred` pair (that re-derived the edges from the swapped blocks) with in-place dup-count adjustment, leaving the edge swap behind. ## Testing Added regression test `Runtime_131716`, covering both the inverted (64-case) and non-inverted bit table shapes. Fails without the fix, passes with it. The original PR also ran SPMI: no asm diffs on benchmarks.run, libraries.pmi and libraries.crossgen2 (~660K contexts). ## Risk Low. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c492a8b-6607-48dc-8a2e-98128e3197b0
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #131716
TryLowerSwitchToBitTestinverts the bit table on xarch when its upper 32 bits are all set, so that it still fits in a 32 bit immediate, and swaps the two targets. The swap only updatedbbCase0/bbCase1, but everything after it consumescase0Edge/case1Edge- soSetCondwired the JCC to the wrong successor, and the dup-count fixup decrementedbbRefson the wrong blocks. Regressed by #116933, which replaced thefgRemoveAllRefPreds/fgAddRefPredpair (which re-derived the edges from the swapped blocks) with in-place dup-count adjustment.Fix: swap the edges and derive the blocks afterwards.
The inversion can only trigger with exactly 64 bit table entries — below that, bits above
bitCountare zero, so~bitTableexceedsUINT32_MAX. That is why only 64-case switches were affected.Before (64 cases, indices 1-3 -> 111):
After:
No SPMI asm diffs on benchmarks.run, libraries.pmi and libraries.crossgen2 (~660K contexts).