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

Stop producing and handling ADDR nodes #78246

Merged
merged 6 commits into from
Dec 15, 2022

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Nov 11, 2022

With this change, the GT_ADDR oper (for a short time before its eventual deletion) becomes "synthetic": no actual nodes of it will be created.

We also start using local address nodes in more places and delete dead code.

Diffs: minor GC info size reductions (LCL_VAR_ADDRs are typed I_IMPL); some substitution diffs (due to fewer GLOB_REFs).

There are nice TP improvements: 0.3% on 64 bit platforms, 1% on 32 bit platforms, of which the latter comes mostly from the OperIsIndir fix.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Nov 11, 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 Nov 11, 2022
@ghost
Copy link

ghost commented Nov 11, 2022

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

Issue Details

Mostly just dead code and cases where we can use LCL_VAR_ADDR.

Some positive diffs are expected from the MD array import change.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

@SingleAccretion SingleAccretion force-pushed the No-Addr-Node-Misc branch 2 times, most recently from 8d332c6 to bcef6eb Compare November 12, 2022 14:38
@SingleAccretion SingleAccretion force-pushed the No-Addr-Node-Misc branch 2 times, most recently from d0d8618 to bcef6eb Compare November 12, 2022 19:04
@SingleAccretion SingleAccretion changed the title Delete some ADDR-related code Stop producing ADDR nodes Dec 7, 2022
@SingleAccretion SingleAccretion changed the title Stop producing ADDR nodes Stop producing and handling ADDR nodes Dec 7, 2022
@SingleAccretion SingleAccretion force-pushed the No-Addr-Node-Misc branch 2 times, most recently from d09f03c to 5b22300 Compare December 8, 2022 17:33
@SingleAccretion SingleAccretion force-pushed the No-Addr-Node-Misc branch 2 times, most recently from ca22141 to e62d5fc Compare December 10, 2022 13:42
@SingleAccretion SingleAccretion marked this pull request as ready for review December 10, 2022 19:00
@SingleAccretion
Copy link
Contributor Author

SingleAccretion commented Dec 10, 2022

@dotnet/jit-contrib

We are getting eerily close to ADDR going completely.

@BruceForstall
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

@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.

Changes LGTM. Thanks for your continuing valuable contributions!

I kicked off stress runs.

@BruceForstall
Copy link
Member

Diffs

@BruceForstall
Copy link
Member

libraries-jitstress had a few failures: one known, one looked like infra, Linux/arm tailcallstress had a "new" crash in System.Memory.Tests. Set to rerun to see if it repros.

@BruceForstall
Copy link
Member

Didn't repro; all failures are "known".

@BruceForstall BruceForstall merged commit 381c782 into dotnet:main Dec 15, 2022
@SingleAccretion SingleAccretion deleted the No-Addr-Node-Misc branch December 15, 2022 17:11
@ghost ghost locked as resolved and limited conversation to collaborators Jan 14, 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