Skip to content
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.

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

Fold all local addresses in local morph #79194

Merged
merged 10 commits into from
Dec 6, 2022

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Dec 3, 2022

This change enables folding for all local address trees in local morph. This is a requirement for the next round of simplifications enabled by the ADDR work (e. g. #78246 depends on this).

There is some amount of (mostly positive) diffs due to more forward substitutions enabled by the fact LCL_ADDR nodes by themselves do not need GLOB_REF. Some regressions in tests due to a bit more GLOB_REFing for "wide" indirs. One case in a huge test method where swapping LCL_VAR and LCL_VAR_ADDR operands for a relop leads us to miss out on a lot of mov suppression.

There is also a very nice TP win, up to 1% reduction in instructions retired.

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

ghost commented Dec 3, 2022

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

Issue Details

This change enables folding for all local address trees in morph. This is a requirement for the next round of simplifications enabled by the ADDR work.

There is some amount of (mostly positive) diffs due to more forward substitutions enabled by the fact LCL_ADDR nodes by themselves do not need GLOB_REF.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

@SingleAccretion SingleAccretion force-pushed the LclMorph-LclAddr-Upstream branch from 29df4b1 to 907a975 Compare December 4, 2022 13:48
@SingleAccretion SingleAccretion force-pushed the LclMorph-LclAddr-Upstream branch from 907a975 to e7a3efa Compare December 4, 2022 13:51
@SingleAccretion SingleAccretion marked this pull request as ready for review December 4, 2022 16:10
@SingleAccretion
Copy link
Contributor Author

@dotnet/jit-contrib

@jakobbotsch This should solve the problem of return buffer calls not having defined address shape before morph.

@BruceForstall
Copy link
Member

fyi, I reran the diffs/replay pipelines because they failed due to missing osx-arm64 MCH files. There should be some there now (but we're still missing libraries-pmi, which failed during collection)

@jakobbotsch
Copy link
Member

Looks great to me! I will run some stress jobs.

@jakobbotsch
Copy link
Member

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

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@SingleAccretion
Copy link
Contributor Author

Stress failures so far: #78898, #58699 (present in the run just before the current one as well).

Libraries stress failures: comparison point is https://dev.azure.com/dnceng-public/public/_build/results?buildId=102920&view=results (the previous run), Number_AsCollectionElement_RoundTrip & System.Collections.Generic.Tests.EqualityComparerTests failed there too.

@jakobbotsch jakobbotsch merged commit e1081df into dotnet:main Dec 6, 2022
@SingleAccretion SingleAccretion deleted the LclMorph-LclAddr-Upstream branch December 7, 2022 17:28
@ghost ghost locked as resolved and limited conversation to collaborators Jan 6, 2023
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.

3 participants