Skip to content

JIT: Preserve RHS writes in SIMD partial stores - #135422

Open
EgorBo with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-jit-bug-helper-vector4
Open

EgorBo with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-jit-bug-helper-vector4

Conversation

Copilot AI commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

For v.X = Helper(ref v), morphing could read the full vector before the RHS, then overwrite the callee’s writes to other fields.

  • Morphing: Keep direct partial stores when the RHS has persistent or ordering side effects. Retain WithElement for side-effect-free RHS expressions.
  • Coverage: Add numerics-field and SIMD-half regressions, plus assignment, pure-RHS, and volatile-read cases.

@azure-pipelines

azure-pipelines Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 5 pipeline(s).
11 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: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Copilot AI and others added 2 commits October 8, 2026 14:13
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix bug with Helper altering Vector4 local JIT: Preserve RHS writes in SIMD partial stores Oct 8, 2026
Copilot AI requested a review from EgorBo October 8, 2026 14:21
@EgorBo
EgorBo marked this pull request as ready for review October 8, 2026 14:24
@EgorBo
EgorBo requested a balanced review from Copilot October 8, 2026 14:24

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.

🔵 Needs a closer look

Cross-architecture code-generation and register-allocation effects need human review and runtime validation; no builds or tests were run here.

0 open findings

What changed in this PR

Addresses #135392 by preserving right-hand-side writes when the JIT morphs partial SIMD stores.

Changes:

  • Keeps direct partial stores for expressions with persistent or ordering side effects.
  • Retains WithElement optimization for side-effect-free expressions.
  • Adds numerics-field, SIMD-half, assignment, pure-expression, and volatile-read regressions.
File Description
src/​tests/​JIT/​Regression_ro_2/​Runtime_135392.cs Adds partial-store regression coverage.
src/​coreclr/​jit/​lclmorph.cpp Prevents unsafe partial-store transformations.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread src/coreclr/jit/lclmorph.cpp Outdated
if (varTypeIsSIMD(varDsc))
{
// Preserve RHS side effects before reading the vector for a partial store.
if (isDef && ((indir->Data()->gtFlags & (GTF_PERSISTENT_SIDE_EFFECTS | GTF_ORDER_SIDEEFF)) != 0))

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.

Do we not need to consider GTF_EXCEPT? We notably now have GTF_OBS_EFFECT which is GTF_SIDE_EFFECT | GTF_ORDER_SIDEEFF (which is therefore GTF_PERSISTENT_SIDE_EFFECTS | GTF_EXCEPT | GTF_ORDER_SIDEEFF)

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.

Yeah, let's use GTF_OBS_EFFECT here then

@tannergooding tannergooding 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. Couple similar issues tracked under #135432 and then some others you logged as well (#134893 is directly related, #135393 and #135394 are indirectly related)

This branch has not been deployed

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

JIT: (bug) v.X = Helper(ref v) on a Vector4 local loses the callee's write to v.Y

4 participants