Skip to content

Conversation

@AndyAyersMS
Copy link
Member

@AndyAyersMS AndyAyersMS commented Sep 9, 2022

In #74384 I modified morph to generate assertions before a block op was morphed, but after the source and dest were morphed. This misses generating some assertions that arise once the expanded form is known.

We now re-run assertion gen on the expanded tree, when we do an OneAsgBlock expansion. Other cases my also prove profitable.

Closes #75229.

Diffs

In dotnet#74384 I modified morph to generate assertions before a block op
was morphed, but after the source and dest were morphed. This misses
generating some assertions that arise once the expanded form is
known.

We now re-run assertion gen on the expanded tree, when we do an
`OneAsgBlock` expansion. Other cases my also prove profitable.

Closes dotnet#75229.
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 9, 2022
@ghost ghost assigned AndyAyersMS Sep 9, 2022
@ghost
Copy link

ghost commented Sep 9, 2022

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

Issue Details

In #74384 I modified morph to generate assertions before a block op was morphed, but after the source and dest were morphed. This misses generating some assertions that arise once the expanded form is known.

We now re-run assertion gen on the expanded tree, when we do an OneAsgBlock expansion. Other cases my also prove profitable.

Closes #75229.

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS
Copy link
Member Author

@kunalspathak PTAL
cc @dotnet/jit-contrib

Copy link
Contributor

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM

@AndyAyersMS
Copy link
Member Author

Note the two methods that were regressed by #74384 show size increases here. This is from alignment padding; the loop body is actually smaller than before.

Top method regressions (bytes):
          10 (18.87 % of base) : 36547.dasm - System.Collections.IndexerSetReverse`1[System.__Canon]:Array():<unnamed>:this
           8 (14.04 % of base) : 32204.dasm - System.Collections.IndexerSet`1[System.__Canon]:Array():<unnamed>:this

@AndyAyersMS AndyAyersMS merged commit 8101ace into dotnet:main Sep 9, 2022
@kunalspathak
Copy link
Contributor

This is from alignment padding; the loop body is actually smaller than before.

Yep, I downloaded and double checked that. :)

image

@ghost ghost locked as resolved and limited conversation to collaborators Oct 10, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JIT: perf regression from improved local struct copy prop

2 participants