Skip to content

[release/10.0] JIT: don't propagate promoted struct LCL_VAR into FIELD_LIST uses#130322

Merged
JulieLeeMSFT merged 5 commits into
release/10.0from
backport/pr-128375-to-release/10.0
Jul 20, 2026
Merged

[release/10.0] JIT: don't propagate promoted struct LCL_VAR into FIELD_LIST uses#130322
JulieLeeMSFT merged 5 commits into
release/10.0from
backport/pr-128375-to-release/10.0

Conversation

@github-actions

@github-actions github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Backport of #128375 to release/10.0

/cc @EgorBo

Customer Impact

  • Customer reported
  • Found internally

Crash. A [StructLayout(LayoutKind.Explicit)] struct that aliases SIMD (Numerics/Vector*) fields with regular fields could make the JIT propagate a promoted struct LCL_VAR into a FIELD_LIST use, causing a segfault in LinearScan::processBlockStartLocations under Release/optimized codegen (#128373).

Regression

  • Yes
  • No

Does not reproduce on .NET 8.

Testing

Added regression test Runtime_128373 (segfaults without the fix, passes with it). Very few SPMI diffs.

Risk

Low, quite a niche scenario.

Note

This PR description was generated with the help of GitHub Copilot.

Copilot AI and others added 5 commits July 7, 2026 22:18
Fixes #128373.

Under tiered compilation with tiered PGO on the SysV x64 ABI, the JIT
could produce an illegal GT_LCL_VAR of a promoted (non-DNER) SIMD local
inside a GT_FIELD_LIST entry of a multi-reg return, hitting an assert
in Lowering::CheckNode in Checked builds and segfaulting LSRA's
processBlockStartLocations in Release builds.

Two phases could introduce the bad IR:

* Local copy-assertion propagation in Morph - Global, propagating a
  `V_dest == V_src` assertion created by MorphCopyBlock onto a later
  whole-struct LCL_VAR use of V_dest, rewriting it to V_src where V_src
  is a promoted (non-DNER) SIMD local.

* Forward substitution (including the one Physical Promotion runs
  inline), substituting a STORE_LCL_VAR's source promoted SIMD LCL_VAR
  directly into a FIELD_LIST entry that no later phase repairs.

Both sites now bail out when the source is a promoted, non-DNER SIMD
local; restricting to SIMD keeps the guards narrow enough to avoid
codegen impact in benchmarks/coreclr_tests SPMI collections.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…promoted SIMD locals

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reframe as a heuristic to avoid DNER when forward substituting whole-local uses into contexts (e.g. GT_FIELD_LIST) that don't support them, and drop the inaccurate stack-home / Lowering::CheckNode wording. No functional change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
@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 Jul 7, 2026
@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.

@JulieLeeMSFT JulieLeeMSFT added this to the 10.0.x milestone Jul 8, 2026

@JulieLeeMSFT JulieLeeMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@tannergooding

Copy link
Copy Markdown
Member

@JulieLeeMSFT, @EgorBo is this missing servicing-consider/approved?

@JulieLeeMSFT JulieLeeMSFT added the Servicing-consider Issue for next servicing release review label Jul 17, 2026
@JulieLeeMSFT

Copy link
Copy Markdown
Member

Egor will submit an approval request.

@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "5f6ab3326cbadc757c0cad917252464e2f851c86",
  "last_dispatched_base_ref": "release/10.0",
  "last_dispatched_base_sha": "aced99138b1efad4a5361ff68ea1951c860dcbda",
  "last_reviewed_commit": "5f6ab3326cbadc757c0cad917252464e2f851c86",
  "last_reviewed_base_ref": "release/10.0",
  "last_reviewed_base_sha": "aced99138b1efad4a5361ff68ea1951c860dcbda",
  "last_recorded_worker_run_id": "29687176221",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "5f6ab3326cbadc757c0cad917252464e2f851c86",
      "review_id": 4730767536
    }
  ]
}

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Holistic Review

Motivation: Backport of #128375 to release/10.0. Fixes a crash (#128373) where a [StructLayout(LayoutKind.Explicit)] struct aliasing SIMD (Vector*/Numerics) fields with regular fields could make the JIT propagate a promoted (non-DNER) SIMD struct LCL_VAR into a GT_FIELD_LIST use of a multi-reg return. That IR is illegal and segfaults LinearScan::processBlockStartLocations in Release codegen (and asserts in Lowering::CheckNode in Checked builds). Customer-reported regression that does not repro on .NET 8, so it is appropriate release-branch material.

Approach: Guards are added at the two phases that could introduce the bad IR: copy-assertion propagation (optCopyAssertionProp) and forward substitution (fgForwardSubStatement) now bail out when the source is a promoted, non-DNER SIMD local. Additionally, fgMorphTree's GT_FIELD_LIST handling marks any whole promoted SIMD LCL_VAR operand as do-not-enregister (dependent promotion, DoNotEnregisterReason::SimdUserForcesDep) to preserve the post-morph promoted-struct invariant. Restricting the guards to SIMD locals keeps them narrow enough to avoid measurable codegen impact per the SPMI collections cited in the original PR. A targeted Runtime_128373 regression test reproduces under TieredCompilation + TieredPGO.

Summary: LGTM. The three JIT source hunks are byte-for-byte identical to the added lines of the original merged commit 03389cb, and the regression test and csproj match the original as well. The change is minimal, well-scoped, and low risk. The reasoning correctly targets both propagation sites plus the morph-time invariant repair. The regression test uses DebugType=None/Optimize=True with the tiered-PGO env vars needed to exercise the optimized path, and asserts a non-NaN result across 300 iterations, which is a reasonable proxy given the crash cannot be asserted directly. No actionable findings.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 47 AIC · ⌖ 14.5 AIC · ⊞ 10K

@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jul 20, 2026
@rbhanda rbhanda modified the milestones: 10.0.x, 10.0.12 Jul 20, 2026
@JulieLeeMSFT
JulieLeeMSFT merged commit 3469f66 into release/10.0 Jul 20, 2026
123 of 126 checks passed
@JulieLeeMSFT
JulieLeeMSFT deleted the backport/pr-128375-to-release/10.0 branch July 20, 2026 20:52
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 Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants