Move vzeroupper emit back to JIT #115748
Merged
Move vzeroupper emit back to JIT #115748
Conversation
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
jkotas
added
the
area-CodeGen-coreclr
There was a problem hiding this comment.
Pull Request Overview
This PR shifts the emission of the VZEROUPPER AVX instruction from static assembly helpers into the JIT pipeline, ensuring it’s only generated when needed.
- Removed unconditional
vzeroupperfrom various static ASM/S assembly helper prologs and epilogs. - Introduced
genClearAvxStatePrologandgenClearAvxStateEpilogin the JIT to emitvzeroupperconditionally. - Updated calls to preserve/restore callee-saved float registers to use new parameterless signatures.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vm/amd64/asmhelpers.S | Removed vzeroupper in static helper macros |
| vm/amd64/AsmHelpers.asm | Removed duplicate vzeroupper in assembly prologs |
| nativeaot/Runtime/amd64/ExceptionHandling.asm | Removed vzeroupper from native AOT funclet helpers |
| nativeaot/Runtime/amd64/ExceptionHandling.S | Same removal in .S variant |
| jit/codegenxarch.cpp | Added JIT hooks (genClearAvxState*) and refactored preserve/restore calls |
| jit/codegencommon.cpp | Inserted genClearAvxStateProlog before float-reg save |
| jit/codegen.h | Declared new clear-state methods and updated signatures |
Comments suppressed due to low confidence (2)
src/coreclr/jit/codegencommon.cpp:5288
- The new
genClearAvxStatePrologbranch logic (checkingContainsCallNeedingVzerouppervs.Contains256bitOrMoreAVX) should have unit or integration tests to validate both code paths emit (or skip)vzeroupperas expected.
genClearAvxStateProlog();
src/coreclr/jit/codegenxarch.cpp:11040
- There are two calls to
genClearAvxStateProlog()in the samegenFuncletPrologimplementation (around lines 10918 and 11040). Consider consolidating to a single insertion point to avoid duplicated VZEROUPPER emission.
genClearAvxStateProlog();
This comment was marked as resolved.
This comment was marked as resolved.
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
vzeroupper is AVX instruction and so it cannot be executed unconditionally in static asm helpers Fixes dotnet#115672
jkotas
commented
May 19, 2025
| int shiftByValue = (int)shiftBy->AsIntConCommon()->IconValue(); | ||
|
|
||
| if (tree->OperIsRotate() && compiler->compOpportunisticallyDependsOn(InstructionSet_BMI2) && !tree->gtSetFlags()) | ||
| if (tree->OperIsRotate() && compiler->compOpportunisticallyDependsOn(InstructionSet_BMI2) && |
tannergooding
approved these changes
May 20, 2025
Open
3 tasks
|
/ba-g unrelated timeout |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
vzeroupper is AVX instruction and so it cannot be executed unconditionally in static asm helpers
Fixes #115672