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

Consolidate morphing of local nodes #69819

Merged
merged 2 commits into from
Jun 4, 2022

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented May 25, 2022

This change creates a new entrypoint for morphing of all local nodes - fgMorphLocal, and utilizes it to remove the restriction on morphing of "x86 stack args under varargs" in LocalAddressVisitor, in preparation of doing the same with implicit byrefs.

Diffs: we have some TP regressions; I have traced them down to MSVC not inlining things, e. g. if I put FORCEINLINE on fgMorphLeaf, I get a negative PIN diff overall. I hope our PGO builds will make up for this. Additionally, the upcoming implicit byref change has some very nice TP wins attached to it.

We also have (expected) GC info diffs:

-      lea      edx, bword ptr [ebp+08H]
-      ; byrRegs +[edx]
+      lea      edx, [ebp+08H]
       call     [hackishModuleName:hackishMethodName()]
-      ; byrRegs -[edx]

The reason is that we now transform the address of the varargs cookie itself to LCL_VAR_ADDR:

-               [000004] ----------- arg1                    \--*  ADDR      byref
-               [000003] -------N---                            \--*  LCL_VAR   int   (AX) V01 VarArgHandle
+               [000004] ----------- arg1                    \--*  LCL_VAR_ADDR byref  V01 VarArgHandle

And so the rationalizer doesn't retype it back to TYP_BYREF:

 *************** Starting PHASE Rationalize IR
-Rewriting GT_ADDR(GT_LCL_VAR) to GT_LCL_VAR_ADDR:
-N002 (  3,  2) [000003] -------N---                    t3 =    LCL_VAR_ADDR byref  V01 VarArgHandle

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

ghost commented May 25, 2022

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

Issue Details

This change creates a new entrypoint for morphing of all local nodes - fgMorphLocal, and utilizes it to remove the restriction on morphing of "x86 stack args under varargs" in LocalAddressVisitor, in preparation of doing the same with implicit byrefs.

We're not expecting diffs.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@SingleAccretion SingleAccretion force-pushed the Local-Morhing-Refactor branch 2 times, most recently from 8fef2c3 to 989a624 Compare May 26, 2022 19:40
Support all types of local nodes in preparation for allowing
varargs stack args to be transformed by LocalAddressVisitor.
@SingleAccretion SingleAccretion force-pushed the Local-Morhing-Refactor branch 2 times, most recently from d004477 to f86aa58 Compare May 28, 2022 15:54
@SingleAccretion SingleAccretion force-pushed the Local-Morhing-Refactor branch from f86aa58 to 84df481 Compare May 28, 2022 15:54
@SingleAccretion SingleAccretion marked this pull request as ready for review May 28, 2022 23:21
@SingleAccretion
Copy link
Contributor Author

SingleAccretion commented May 28, 2022

Build windows x64 debug NativeAOT failed with what looks like #69192. There is also a timeout; not sure what it is about, but seems quite unlikely to be related to this almost zero-diff change.

@dotnet/jit-contrib

@jakobbotsch jakobbotsch merged commit 3cfd7a0 into dotnet:main Jun 4, 2022
@SingleAccretion SingleAccretion deleted the Local-Morhing-Refactor branch June 4, 2022 16:15
@ghost ghost locked as resolved and limited conversation to collaborators Jul 4, 2022
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