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

JIT: Handle primitive remainder stores to regularly promoted fields in physical promotion #87217

Merged
merged 2 commits into from Jun 8, 2023

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Jun 7, 2023

If the remainder of a block copy is handled as a primitive, then it is possible that the destination or source is a regularly promoted field. In this case we do not have to DNER it.

Example. Before:

STMT00003 ( 0x00D[E-] ... 0x00E )
               [000009] DA---------STORE_LCL_VAR struct<System.Nullable`1, 8>(P) V02 loc1         
                                                            ▌    bool   V02.<unknown class>:hasValue (offs=0x00) -> V08 tmp4         
                                                            ▌    int    V02.<unknown class>:value (offs=0x04) -> V09 tmp5         
               [000008] -----------                         └──▌  LCL_VAR   struct<System.Nullable`1, 8> V00 arg0          (last use)

Processing block operation [000009] that involves replacements
  V08 (field V02.hasValue (fldOffset=0x0)) <- V16 (V00.[000..001)) (last use)
  Remainder: [004..008)
  => Remainder strategy: int at +004

Local V00 should not be enregistered because: was accessed as a local field

Local V02 should not be enregistered because: was accessed as a local field
New statement:
STMT00003 ( 0x00D[E-] ... 0x00E )
               [000090] -A---------COMMA     void
               [000087] DA---------                         ├──▌  STORE_LCL_VAR bool   V08 tmp4
               [000086] -----------                         │  └──▌  LCL_VAR   bool   V16 tmp12         (last use)
               [000089] UA---------                         └──▌  STORE_LCL_FLD int   (P) V02 loc1         [+4]                       // causes DNER for V02 even though this is exactly the second field
                                                               ▌    bool   V02.<unknown class>:hasValue (offs=0x00) -> V08 tmp4
                                                               ▌    int    V02.<unknown class>:value (offs=0x04) -> V09 tmp5
               [000088] -----------                            └──▌  LCL_FLD   int    V00 arg0         [+4]

After:

STMT00003 ( 0x00D[E-] ... 0x00E )
               [000009] DA---------STORE_LCL_VAR struct<System.Nullable`1, 8>(P) V02 loc1         
                                                            ▌    bool   V02.<unknown class>:hasValue (offs=0x00) -> V08 tmp4         
                                                            ▌    int    V02.<unknown class>:value (offs=0x04) -> V09 tmp5         
               [000008] -----------                         └──▌  LCL_VAR   struct<System.Nullable`1, 8> V00 arg0          (last use)

Processing block operation [000009] that involves replacements
  V08 (field V02.hasValue (fldOffset=0x0)) <- V16 (V00.[000..001)) (last use)
  Remainder: [004..008)
  => Remainder strategy: int at +004

Local V00 should not be enregistered because: was accessed as a local field
New statement:
STMT00003 ( 0x00D[E-] ... 0x00E )
               [000090] -A---------COMMA     void
               [000087] DA---------                         ├──▌  STORE_LCL_VAR bool   V08 tmp4                        // No more DNER
               [000086] -----------                         │  └──▌  LCL_VAR   bool   V16 tmp12         (last use)
               [000089] DA---------                         └──▌  STORE_LCL_VAR int    V09 tmp5
               [000088] -----------                            └──▌  LCL_FLD   int    V00 arg0         [+4]

…n physical promotion

If the remainder of a block copy is handled as a primitive, then it is
possible that the destination or source is a regularly promoted field.
In this case we do not have to DNER it.

Example. Before:

```
Processing block operation [000009] that involves replacements
  V08 (field V02.hasValue (fldOffset=0x0)) <- V16 (V00.[000..001)) (last use)
  Remainder: [004..008)
  => Remainder strategy: int at +004

Local V00 should not be enregistered because: was accessed as a local field

Local V02 should not be enregistered because: was accessed as a local field
New statement:
STMT00003 ( 0x00D[E-] ... 0x00E )
               [000090] -A---------                         ▌  COMMA     void
               [000087] DA---------                         ├──▌  STORE_LCL_VAR bool   V08 tmp4
               [000086] -----------                         │  └──▌  LCL_VAR   bool   V16 tmp12         (last use)
               [000089] UA---------                         └──▌  STORE_LCL_FLD int   (P) V02 loc1         [+4]
                                                               ▌    bool   V02.<unknown class>:hasValue (offs=0x00) -> V08 tmp4
                                                               ▌    int    V02.<unknown class>:value (offs=0x04) -> V09 tmp5
               [000088] -----------                            └──▌  LCL_FLD   int    V00 arg0         [+4]
```

After:

```
Processing block operation [000009] that involves replacements
  V08 (field V02.hasValue (fldOffset=0x0)) <- V16 (V00.[000..001)) (last use)
  Remainder: [004..008)
  => Remainder strategy: int at +004

Local V00 should not be enregistered because: was accessed as a local field
New statement:
STMT00003 ( 0x00D[E-] ... 0x00E )
               [000090] -A---------                         ▌  COMMA     void
               [000087] DA---------                         ├──▌  STORE_LCL_VAR bool   V08 tmp4
               [000086] -----------                         │  └──▌  LCL_VAR   bool   V16 tmp12         (last use)
               [000089] DA---------                         └──▌  STORE_LCL_VAR int    V09 tmp5
               [000088] -----------                            └──▌  LCL_FLD   int    V00 arg0         [+4]

```
@ghost ghost assigned jakobbotsch Jun 7, 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 Jun 7, 2023
@ghost
Copy link

ghost commented Jun 7, 2023

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

Issue Details

If the remainder of a block copy is handled as a primitive, then it is possible that the destination or source is a regularly promoted field. In this case we do not have to DNER it. For example

Example. Before:

STMT00003 ( 0x00D[E-] ... 0x00E )
               [000009] DA---------STORE_LCL_VAR struct<System.Nullable`1, 8>(P) V02 loc1         
                                                            ▌    bool   V02.<unknown class>:hasValue (offs=0x00) -> V08 tmp4         
                                                            ▌    int    V02.<unknown class>:value (offs=0x04) -> V09 tmp5         
               [000008] -----------                         └──▌  LCL_VAR   struct<System.Nullable`1, 8> V00 arg0          (last use)

Processing block operation [000009] that involves replacements
  V08 (field V02.hasValue (fldOffset=0x0)) <- V16 (V00.[000..001)) (last use)
  Remainder: [004..008)
  => Remainder strategy: int at +004

Local V00 should not be enregistered because: was accessed as a local field

Local V02 should not be enregistered because: was accessed as a local field
New statement:
STMT00003 ( 0x00D[E-] ... 0x00E )
               [000090] -A---------COMMA     void
               [000087] DA---------                         ├──▌  STORE_LCL_VAR bool   V08 tmp4
               [000086] -----------                         │  └──▌  LCL_VAR   bool   V16 tmp12         (last use)
               [000089] UA---------                         └──▌  STORE_LCL_FLD int   (P) V02 loc1         [+4]
                                                               ▌    bool   V02.<unknown class>:hasValue (offs=0x00) -> V08 tmp4
                                                               ▌    int    V02.<unknown class>:value (offs=0x04) -> V09 tmp5
               [000088] -----------                            └──▌  LCL_FLD   int    V00 arg0         [+4]

After:

STMT00003 ( 0x00D[E-] ... 0x00E )
               [000009] DA---------STORE_LCL_VAR struct<System.Nullable`1, 8>(P) V02 loc1         
                                                            ▌    bool   V02.<unknown class>:hasValue (offs=0x00) -> V08 tmp4         
                                                            ▌    int    V02.<unknown class>:value (offs=0x04) -> V09 tmp5         
               [000008] -----------                         └──▌  LCL_VAR   struct<System.Nullable`1, 8> V00 arg0          (last use)

Processing block operation [000009] that involves replacements
  V08 (field V02.hasValue (fldOffset=0x0)) <- V16 (V00.[000..001)) (last use)
  Remainder: [004..008)
  => Remainder strategy: int at +004

Local V00 should not be enregistered because: was accessed as a local field
New statement:
STMT00003 ( 0x00D[E-] ... 0x00E )
               [000090] -A---------COMMA     void
               [000087] DA---------                         ├──▌  STORE_LCL_VAR bool   V08 tmp4
               [000086] -----------                         │  └──▌  LCL_VAR   bool   V16 tmp12         (last use)
               [000089] DA---------                         └──▌  STORE_LCL_VAR int    V09 tmp5
               [000088] -----------                            └──▌  LCL_FLD   int    V00 arg0         [+4]
Author: jakobbotsch
Assignees: jakobbotsch
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch jakobbotsch closed this Jun 8, 2023
@jakobbotsch jakobbotsch reopened this Jun 8, 2023
@jakobbotsch jakobbotsch mentioned this pull request Jun 8, 2023
40 tasks
@jakobbotsch jakobbotsch marked this pull request as ready for review June 8, 2023 14:41
@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @EgorBo.

Diffs with physical promotion. Quite large x86/arm32 diffs compared to 64-bit, not quite sure why, will try to take a closer look, and also look at some of the regressions.

Diffs without old promotion. None as expected since there are no regular promoted fields in this case.

@jakobbotsch jakobbotsch requested a review from EgorBo June 8, 2023 14:43
@jakobbotsch
Copy link
Member Author

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

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jakobbotsch
Copy link
Member Author

Quite large x86/arm32 diffs compared to 64-bit, not quite sure why, will try to take a closer look

We only promote struct parameters if they are passed in registers or as implicit byrefs, which particularly means a parameter Span<T> is never regularly promoted on x86, so physical promotion finds lots of promotion opportunities for these cases. And these are commonly copied to non-parameter locals that are regularly promoted, in which case this change often kicks in (in all the cases where physical promotion only promoted the span byref or length, but not both fields).

also look at some of the regressions.

There are very few, but the one I looked at was just due to different register allocation because of a new independent promotion taking up a register and another local then being spilled instead.

@jakobbotsch
Copy link
Member Author

Failures look like #84006, #84911 and timeouts from CI being in a bad state.

@jakobbotsch jakobbotsch merged commit 3fded0b into dotnet:main Jun 8, 2023
179 of 192 checks passed
@jakobbotsch jakobbotsch deleted the remainder-promoted branch June 8, 2023 20:20
@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants