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

Delete GT_ADDR #84147

Merged
merged 1 commit into from
Apr 1, 2023
Merged

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Mar 30, 2023

The real work has at this point been done, so this just deletes the oper and its last use as a sentinel. No diffs.

Some TODO-ADDRs in source still remain, but they can be addressed separately.

Closes #11057.

With this legacy IR construct gone, let's see how much time will it take to delete its ASG cousin :).

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

ghost commented Mar 30, 2023

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

Issue Details

The real work has at this point been done, so this just deletes the oper and its last use as a sentinel.

Some TODO-ADDRs in source still remain, but they can be addresses separately.

Closes #11057. No diffs expected.

With one legacy IR construct gone, let's see how much time will it take to delete its ASG cousin :).

Depends on #79722.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

@AndyAyersMS
Copy link
Member

And they said this day would never come...

Glad to see this at long last.

@SingleAccretion SingleAccretion marked this pull request as ready for review March 31, 2023 18:09
@SingleAccretion
Copy link
Contributor Author

@dotnet/jit-contrib

Copy link
Member

@EgorBo EgorBo left a comment

Choose a reason for hiding this comment

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

:shipit: will leave it open for someone else to take another look

@AndyAyersMS AndyAyersMS merged commit 9a8cc5e into dotnet:main Apr 1, 2023
@am11
Copy link
Member

am11 commented Apr 1, 2023

Congrats! 🎉

@SingleAccretion, design docs also mention GT_ADDR in a few places:

$ git grep GT_ADDR
docs/design/coreclr/jit/ryujit-overview.md:| [Rationalization](#rationalization) | Flowgraph order changes from `FGOrderTree` to `FGOrderLinear`. All `GT_COMMA`, `GT_ASG` and `GT_ADDR` nodes are transformed. |
docs/design/coreclr/jit/ryujit-overview.md:* Elimination of address nodes (`GT_ADDR`). These are problematic because of the need for parent context to analyze the child.
docs/design/coreclr/jit/ryujit-overview.md:  * All `GT_ADDR` nodes are eliminated (e.g. with `GT_LCL_VAR_ADDR`).
docs/design/coreclr/jit/ryujit-tutorial.md:    - Eliminate GT_ADDR: Need parent context to analyze child
docs/design/coreclr/jit/ryujit-tutorial.md:We would like to ensure that all data flows are from child to parent. Examples where this is currently violated are in the assignment, or GT_ASG node, where the data flows from the right hand side of the assignment "over" the parent assignment, to the value being defined on the left hand side. Similarly, the child of a GT_ADDR node may appear to be a use or a definition of the value, when instead its address is being taken.
docs/design/coreclr/jit/ryujit-tutorial.md:- All GT_ADDR nodes are eliminated (e.g. with GT_LCL_VAR_ADDR)

@SingleAccretion
Copy link
Contributor Author

Thank you :). ADDR is not the only obsolete concept they mention - #69988.

@SingleAccretion SingleAccretion deleted the Delete-Addr-Upstream branch April 1, 2023 19:05
@ghost ghost locked as resolved and limited conversation to collaborators May 2, 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.

4 participants