[release/10.0] Fix SIMD primitive zero initialization - #133170
[release/10.0] Fix SIMD primitive zero initialization#133170tannergooding wants to merge 1 commit into
Conversation
`TryPrimitiveInit` created a SIMD zero node when converting a zero block initialization into a primitive local store, but the rationalized store retained its original integer-zero data node. This produced a SIMD store with an integer source, leading to invalid codegen and Tier0 compilation failures. The missing source replacement was introduced by the assignment rationalization changes in dotnet#85585 (`53b4cd0912d`), where the legacy `GT_ASG` path updated `gtOp2` but the rationalized store path did not update `Data()`. Before the fix, the `Vector256` case encoded `C4 E1 FD 6E C0` (`vmovq` with `VEX.L=1`). The corrected tree emits a SIMD zero (`vxorps`) in both FullOpts and Tier0. Regression coverage includes `Vector128` and `Vector256` in both modes. Fixes dotnet#133085 > [!NOTE] > This pull request description was generated with GitHub Copilot. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> (cherry picked from commit f0b01ad)
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The JIT fix correctly updates the store operand for SIMD zero-inits and the added regression test directly covers the reported failure modes.
Review tier: Lite
Findings: None
What changed in this PR
Backports the fix for a CoreCLR JIT morphing bug where a SIMD-typed local block zero-init could be converted into a primitive SIMD store while still carrying an integer-zero source node, leading to invalid codegen (illegal instruction) or Tier0 compilation failure; adds a targeted JIT regression test to prevent recurrence on release/10.0.
Changes:
- Fix
MorphInitBlockHelper::TryPrimitiveInitto replace the store’s data operand when retyping an integral zero-init into a SIMD zero. - Add a new JIT regression test (
Runtime_133085) coveringVector128<ulong>andVector256<ulong>in both AggressiveOptimization (FullOpts) and Tier0 scenarios. - Add test project configuration to run with tiered compilation enabled and in isolated process mode.
| File | Description |
|---|---|
| src/coreclr/jit/morphblock.cpp | Ensures SIMD zero-init transforms update the store’s Data() operand to a SIMD-typed zero node (and marks it morphed) before converting to GT_STORE_LCL_VAR. |
| src/tests/JIT/Regression/JitBlue/Runtime_133085/Runtime_133085.cs | Adds regression coverage for zeroing a SIMD local via a reinterpreted struct byref under both FullOpts and Tier0. |
| src/tests/JIT/Regression/JitBlue/Runtime_133085/Runtime_133085.csproj | Adds the test project with process isolation and a tiered compilation environment variable required to exercise the Tier0 path. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
@EgorBo, please review this servicing PR. |
|
@tannergooding, please check a test failure. |
|
Test failures are unrelated, they are the httplistener timeouts and quic failures that have since been fixed in main |
Backport of #133100 to release/10.0
/cc @tannergooding
Customer Impact
Reported in #133085. Zero-initializing a struct through a byref that the JIT can see is the address of a
Vector128<T>/Vector256<T>local (Unsafe.As<Vector256<ulong>, S>(ref v) = default;) produces a SIMD store whose source is an integer zero constant. With optimizations that lowers tovmovq ymm0, rax(C4 E1 FD 6E C0), which has no valid VEX.256 encoding, so the process dies withExecutionEngineException: Illegal instruction; at Tier0 the same method fails to compile and throwsInvalidProgramException. Expected behavior is a SIMD zero (vxorps) in both modes.The pattern shows up in code that reinterprets vector locals as multi-limb structs, and there is no compile-time diagnostic — it fails at runtime on any AVX-capable x64 machine.
Regression
Not a regression in 10.0 — .NET 9 fails the same way. The bad tree dates to the assignment rationalization work in #85585 (
53b4cd0912d), where the legacyGT_ASGpath updatedgtOp2but the rationalized store path never updatedData().Testing
New regression test
src/tests/JIT/Regression/JitBlue/Runtime_133085, coveringVector128<ulong>andVector256<ulong>in both FullOpts (AggressiveOptimization) and Tier0. It reproduces the illegal encoding /InvalidProgramExceptionwithout the fix and passes with it.Missed previously because
TryPrimitiveInitlooked correct in isolation — it built the SIMD zero node and assigned it tom_src— and no existing test zero-initialized a SIMD local through a reinterpreted struct view, so nothing exercised the path where the store's data operand still had to be replaced.Risk
Low. Two lines in
TryPrimitiveInit, reached only when a block zero-init of a SIMD-typed local is converted into a primitive store. It makes the store's data node match the store's type, which is what the transform already intended; every other case was already consistent.This is a manual backport: the cherry-pick conflicted only because the member is named
m_compon release/10.0 rather thanm_compiler. The change is otherwise identical to #133100.Note
This pull request description was generated with GitHub Copilot.