Skip to content

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Dec 16, 2022

The two changes help to fix the CQ gap that exists today between LCL_FLD and IND(LCL_ADDR) forms of access to address-exposed locals.

Positive diffs, with some regressions due to the usual reasons associated with more CSEing (e. g. the large x86 test regressions are actually PerfScore improvements). A bit of a TP regression due to more VNing, will be payed for with #79722.

@ghost ghost added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member labels Dec 16, 2022
@ghost
Copy link

ghost commented Dec 16, 2022

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

Issue Details

The two changes help to fix the CQ gap that exists today between LCL_FLD and IND(LCL_ADDR) forms of access to address-exposed locals.

Positive diffs are expected, with some regressions due to the usual reasons associated with more CSEing.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@SingleAccretion
Copy link
Contributor Author

This will unblock #79722.

@SingleAccretion
Copy link
Contributor Author

@dotnet/jit-contrib

@jakobbotsch
Copy link
Member

/azp run runtime-coreclr superpmi-asmdiffs, runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, Fuzzlyn

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jakobbotsch
Copy link
Member

The failures look like they might be related?

@SingleAccretion
Copy link
Contributor Author

SingleAccretion commented Jan 4, 2023

Taking a look...

Edit: the bug has been identified and fixed.

Good on me for noticing it a few weeks ago in dotnet#79722...

```
// Otherwise, must be local lhs form. TODO-Bug: this doesn't account for LCL_FLD.
// This will miss memory havoc induced by address-exposed local field stores.
```
@jakobbotsch
Copy link
Member

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@jakobbotsch jakobbotsch left a comment

Choose a reason for hiding this comment

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

LGTM!

@jakobbotsch jakobbotsch merged commit d4feb5b into dotnet:main Jan 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 10, 2023
@SingleAccretion SingleAccretion deleted the Blk-Vars branch March 28, 2023 20:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants