Disable AVX/AVX2 on interpreter-only x64 builds to fix Vector256 NRE#129574
Open
kotlarmilos wants to merge 1 commit into
Open
Disable AVX/AVX2 on interpreter-only x64 builds to fix Vector256 NRE#129574kotlarmilos wants to merge 1 commit into
kotlarmilos wants to merge 1 commit into
Conversation
On JIT-less x64 builds such as Mac Catalyst and the iOS or tvOS simulator, code using Vector256<T> can throw a NullReferenceException in Vector256.get_IsHardwareAccelerated. These targets compile R2R against the x86-64-v2 baseline with no AVX, so crossgen2 folds Vector256.IsHardwareAccelerated to false and guards the body with a CHECK_InstructionSetSupport AVX2-unsupported fixup. However SetCpuInfo enables AVX and AVX2 from the host CPU with no no-JIT gate, so on AVX2 hardware that fixup fails, the runtime discards the correct 128-bit R2R body and falls back to the interpreter, which mishandles the 256-bit path and throws the NRE. Clear InstructionSet_AVX when interpreterOnly so the dependency resolver cascade-removes AVX2, AVX512, Vector256 and Vector512 while keeping Vector128 and SSE, re-aligning the reported ISA with the v2 R2R baseline so the fixup validates and the 128-bit path is used. Follow-up to dotnet#129012 that keys off the same interpreterOnly flag as the adjacent Vector<T> cap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates CoreCLR’s CPU ISA reporting on x86/x64 when running in interpreter-only mode by clearing AVX so that subsequent ISA dependency normalization removes AVX2/AVX512-class capabilities. This helps keep runtime-reported ISA support aligned with the ReadyToRun baseline used for interpreter-only x64 targets, avoiding mismatches that can cause incorrect code path selection.
Changes:
- In
EEJitManager::SetCpuInfo(), clearInstructionSet_AVXwheninterpreterOnlyon x86/x64 underFEATURE_INTERPRETER. - Rely on existing ISA dependency normalization (
EnsureValidInstructionSetSupport) to cascade-remove dependent ISAs (e.g., AVX2/AVX512-class).
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/codeman.cpp | Clears AVX for interpreter-only x86/x64 to force consistent ISA dependency resolution and avoid AVX2+ being reported enabled in no-JIT interpreter-only scenarios. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
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.
Description
On interpreter-only x64 builds such as Apple mobile, code using
Vector256<T>can throw aNullReferenceExceptioninVector256.get_IsHardwareAccelerated(). These targets compile R2R against the x86-64-v2 baseline with no AVX, so crossgen2 foldsVector256.IsHardwareAcceleratedtofalseand guards the body with aCHECK_InstructionSetSupportAVX2-unsupported fixup. HoweverSetCpuInfo()enables AVX and AVX2 from the host CPU with no no-JIT gate, so on AVX2 hardware that fixup fails, the runtime discards the correct 128-bit R2R body and falls back to the interpreter, which mishandles the 256-bit path and throws the NRE. The fix clearsInstructionSet_AVXwheninterpreterOnly, and the dependency resolver then cascade-removes AVX2, AVX512, Vector256 and Vector512 while keeping Vector128 and SSE, re-aligning the reported ISA with the v2 R2R baseline so the fixup validates and the 128-bit path is used.This is a follow-up to #129012 that keys off the same
interpreterOnlyflag as the adjacentVector<T>cap, and fixes the runtime side of dotnet/macios#25734.Validation
I compiled CoreLib to R2R for two targets and disassembled
Vector256.get_IsHardwareAcceleratedin each. Onmaccatalyst-x64(v2 baseline) it is hard-coded to return false and carries anAvx2-fixup, meaning the runtime only keeps this body if the CPU has no AVX2. Onlinux-x64(v3 baseline) it is hard-coded to return true with no fixup. This shows the value is baked in at compile time, and that on Apple x64 theAvx2-fixup is what fails on a real AVX2 CPU, causing the runtime to drop the correct body and fall back to the interpreter that crashes.maccatalyst-x64, x86-64-v2 baseline:linux-x64, x86-64-v3 baseline: