Skip to content

JIT: Track AVX state from register-producing widths - #134560

Merged
tannergooding merged 5 commits into
dotnet:mainfrom
tannergooding:tannergooding-local-pr-review
Sep 25, 2026
Merged

tannergooding merged 5 commits into
dotnet:mainfrom
tannergooding:tannergooding-local-pr-review

Conversation

@tannergooding

@tannergooding tannergooding commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Follow up on #134410 by tracking AVX upper state from register-producing widths rather than vector types or memory-store widths. This removes unnecessary vzeroupper instructions while accounting for previously missed wide loads, copies, and integer-division intermediates.

  • Account for allocator-inserted spills/reloads and actual local register copies.
  • Narrow unsafe-widening/extraction copies to the defined source or required result width, and use XMM zeroing in integer-vector division.
  • Preserve the existing call/prolog/epilog clearing policy. Native-boundary policy changes, including the pre-existing unmanaged calli gap, remain out of scope.

SuperPMI results

CI build 1610798, testing head b20f2c3d058:

Target Compared contexts Net code-size change FullOpts contribution
Windows x64 2,814,515 +810 bytes −1,008 bytes
Linux x64 3,256,661 +15,645 bytes +11,970 bytes
Windows x86 2,576,563 +11,088 bytes +8,784 bytes

ARM, ARM64, and WASM have no assembly diffs. Windows corpus coverage expanded substantially, so raw totals are not directly comparable with earlier runs. Remaining growth includes both corrected wide-producer accounting and conservative method-wide cleanup placement; it is not all individually necessary transition cleanup.

JIT executed-instruction counts increased approximately 0.011–0.031% on xarch; these are compiler-work measurements, not application-throughput results. The assembly runs report zero failing compilations, with symmetric missing-context counts between base and diff.

Validated with Checked x64/all-target JIT builds, focused execution and disassembly checks across default/AVX2/AVX/SSE/software configurations, targeted register-stress runs, and JIT formatting. Runtime execution was on Windows x64; cross-target replay is not runtime execution on those platforms.

Note

This PR description and implementation were prepared with GitHub Copilot assistance.

tannergooding and others added 2 commits September 23, 2026 18:45
Account for wide temporaries and allocator-generated reloads while avoiding unnecessary clears for narrow producers and stores. Recognize unmanaged calli boundaries and narrow unsafe vector copies and division zeroing.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Retain the merged block-operation fix and its comments while preserving the additional producer-width tracking and codegen changes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 24, 2026 01:51
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 24, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 6 pipeline(s).
10 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Local SIMD stores still need correction, and focused GetLower128 regression coverage is missing.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Updates x64 JIT AVX-state tracking to reflect actual register-producing widths and unmanaged call boundaries.

Changes:

  • Refines SIMD width tracking, reload handling, and division temporary codegen.
  • Recognizes unmanaged calli boundaries.
  • Adds functional and disassembly regression coverage.
File Reviewed changes
src/​tests/​JIT/​opt/​Vectorization/​AvxState.csproj Configures the AVX-state regression test.
src/​tests/​JIT/​opt/​Vectorization/​AvxState.cs Adds functional and disassembly coverage.
src/​coreclr/​jit/​lsraxarch.cpp Tracks SIMD widths through LSRA.
src/​coreclr/​jit/​lsra.cpp Accounts for allocator reloads and copies.
src/​coreclr/​jit/​hwintrinsiccodegenxarch.cpp Narrows register moves and division zeroing.
src/​coreclr/​jit/​gentree.cpp Handles unmanaged calls requiring AVX-state clearing.

Comment thread src/coreclr/jit/lsraxarch.cpp Outdated
Constrain Vector<T> to 128 bits so Vector512 narrowing exercises GetLower128, and verify the narrow copy preserves both the result and source.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 24, 2026 02:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Review findings identify a critical AVX/AVX2 assertion risk and gaps in tracking and regression coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Test does not verify unmanaged calli vzeroupper behavior

src/​tests/​JIT/​opt/​Vectorization/​AvxState.cs:84

This pattern does not actually verify the new unmanaged-calli behavior: CallUnmanaged necessarily emits an xorps/vxorps to materialize Vector128<float>.Zero, so the alternation can match that instruction even when NeedsVzeroupper still returns false for IsUnmanaged(). The regression would therefore pass against the old JIT; constrain the check to vzeroupper in AVX-enabled runs (and separately cover the no-AVX fallback), or otherwise make the call-site assertion unambiguous.

Comment thread src/coreclr/jit/lsraxarch.cpp
@tannergooding

Copy link
Copy Markdown
Member Author

Regarding the unmanaged calli test finding in the review summary: the leading space in {{vzeroupper| xorps }} is intentional. The xorps alternative matches the legacy instruction used without AVX, but cannot match vxorps.

The preserved pre-fix AVX-enabled disassembly contains vxorps xmm0, xmm0, xmm0 and no vzeroupper; it fails this exact check. The fixed disassembly includes vzeroupper and passes. The test therefore distinguishes the missing unmanaged-call clearing from the no-AVX fallback as intended.

Note

This reply was drafted with GitHub Copilot assistance.

@tannergooding
tannergooding marked this pull request as draft September 24, 2026 04:17
tannergooding and others added 2 commits September 23, 2026 21:43
Keep unmanaged calli clearing at wide-state call sites without broadening prolog clearing. Track local-store register copies after allocation instead of treating stack stores as wide producers.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve existing native-boundary classification and defer unmanaged calli cleanup ownership to a separate change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@tannergooding

Copy link
Copy Markdown
Member Author

@EgorBo for this one it does some more cleanup and fixes other places we were incorrectly handling vzeroupper.

It still doesn't handle every possible case of unmanaged call (such as through calli, certain delegates, or fnptrs) because that caused far too many regressions and needs more complex handling to do properly.

The size regressions that are still present are generally correctness improvements. Ideally we'd have a bit more sophisticated tracked to avoid doing the operation unnecessarily, particularly when there's like only a single YMM usage late or optionally in the method, but that is a much more complex and involved fix.

This at least generally fixes the remaining cases that were trivially incorrectly tracking the state.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved AVX-state accounting gaps remain for wide constants, spill paths without reloads, and regression coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment thread src/coreclr/jit/lsra.cpp
@MichalPetryka

Copy link
Copy Markdown
Contributor

certain delegates

Don't those just use IL stubs with calli? So they should be handled by calli handling cc @jkoritzinsky

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants