Skip to content

JIT: add missing xarch RMW case #92252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Sep 19, 2023
Merged

JIT: add missing xarch RMW case #92252

merged 1 commit into from
Sep 19, 2023

Conversation

AndyAyersMS
Copy link
Member

Handle the case where we're indirectly updating a local with a value that is not a constant.

Fixes #92218.

@AndyAyersMS
JIT: add missing xarch RMW case
edad3b9 
Handle the case where we're indirectly updating a local with a value
that is not a constant.

Fixes dotnet#92218.
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 18, 2023
@ghost ghost assigned AndyAyersMS Sep 18, 2023
@ghost
Copy link

ghost commented Sep 18, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Handle the case where we're indirectly updating a local with a value that is not a constant.

Fixes #92218.

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr
Milestone: -

@AndyAyersMS
Copy link
Member Author

@BruceForstall PTAL
cc @dotnet/jit-contrib

This is a regression from .NET 7, so we'll need to make the case for a backport.

BruceForstall
BruceForstall approved these changes Sep 19, 2023
View reviewed changes
Copy link
Contributor

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

LGTM

Probably should run jitstress

@AndyAyersMS
Copy link
Member Author

/azp run runtime-coreclr jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@build-analysis build-analysis bot mentioned this pull request Sep 19, 2023
Closed
@jakobbotsch
Copy link
Member

The issue looks like the same as #41073, where it seems we elected not to fix it in emitHandleMemOp. Does the reasoning given in its PR make sense for this situation too?

@AndyAyersMS AndyAyersMS mentioned this pull request Sep 19, 2023
Closed
@AndyAyersMS
Copy link
Member Author

The issue looks like the same as #41073, where it seems we elected not to fix it in emitHandleMemOp. Does the reasoning given in its PR make sense for this situation too?

My fix is in emitInsRMW so is consistent with #50669 (the fix for #41073), which added the S_I case.

jitstress failures also happening in main (#92202 ).

@AndyAyersMS
Copy link
Member Author

Build analysis doesn't seem to be tracking known jitstress issues, so will have to "merge on red".

@AndyAyersMS AndyAyersMS merged commit 67dbbeb into dotnet:main Sep 19, 2023
@AndyAyersMS
Copy link
Member Author

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6239547972

@github-actions github-actions bot mentioned this pull request Sep 19, 2023
Merged
@jakobbotsch
Copy link
Member

Build analysis doesn't seem to be tracking known jitstress issues, so will have to "merge on red".

The issues with the JSON blobs need to have the "Known Build Error" label to be included in the analysis. I've added it to #92202

@AndyAyersMS
Copy link
Member Author

Build analysis doesn't seem to be tracking known jitstress issues, so will have to "merge on red".

The issues with the JSON blobs need to have the "Known Build Error" label to be included in the analysis. I've added it to #92202

Aha. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BruceForstall BruceForstall BruceForstall approved these changes

Assignees

@AndyAyersMS AndyAyersMS

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect NRE during operations on structures in method marked with AgressiveOptimization
3 participants
@AndyAyersMS @jakobbotsch @BruceForstall