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: Cache memory dependencies for VN map selects #89241

Merged
merged 6 commits into from
Jul 20, 2023

Conversation

jakobbotsch
Copy link
Member

Hoisting relies on being able to look at which memory definitions a candidate is dependent upon. VN tracks these during the map select logic; however, it fails to do so when the map selection hits the cache. This changes the logic to make sure the memory dependencies are cached.

Fix #75442

TP impact of the "obvious" approach was not as bad as I expected; below 0.1%.

Diff of memory stats over win-x64 aspnet. This costs about 1% more ValueNumber memory, which amounts to about 0.09% more overall memory usage.

Note: this is best reviewed with whitespace diffs turned off as I have unnested some code in VNForMapSelectWork.

Hoisting relies on being able to look at which memory dependencies a
candidate is dependent upon. VN tracks these during the map select
logic; however, it fails to do so when the map selection hits the cache.
This changes the logic to make sure the memory dependencies are cached.

Fix dotnet#75442
@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 Jul 20, 2023
@ghost ghost assigned jakobbotsch Jul 20, 2023
@ghost
Copy link

ghost commented Jul 20, 2023

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

Issue Details

Hoisting relies on being able to look at which memory definitions a candidate is dependent upon. VN tracks these during the map select logic; however, it fails to do so when the map selection hits the cache. This changes the logic to make sure the memory dependencies are cached.

Fix #75442

TP impact of the "obvious" approach was not as bad as I expected; below 0.1%.

Diff of memory stats over win-x64 aspnet. This costs about 1% more ValueNumber memory, which amounts to about 0.09% more overall memory usage.

Note: this is best reviewed with whitespace diffs turned off as I have unnested some code in VNForMapSelectWork.

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch jakobbotsch marked this pull request as ready for review July 20, 2023 20:25
@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs. Some minor TP regressions (up to +0,07%). Memory regressions can be seen above.

@jakobbotsch jakobbotsch merged commit 8c730dd into dotnet:main Jul 20, 2023
135 checks passed
@jakobbotsch jakobbotsch deleted the fix-75442 branch July 20, 2023 21:04
@ghost ghost locked as resolved and limited conversation to collaborators Aug 20, 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.

JIT: Bad codegen with loop
2 participants