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 lvVerTypeInfo #71597

Merged
merged 1 commit into from
Jul 23, 2022
Merged

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Jul 3, 2022

Saves 0.7% in memory consumption when CG-ing CoreLib.

We're mostly not expecting diffs, except a few where the new "is normed" logic in fgFindJumpTargets leads to us to inline something we didn't before. The reason this happens is that previously we would assign "is normed" false in case the inlining info had TI_ERROR (invalid) lclVerTypeInfos in it.

Diffs - there is a large TP regression on x86.

I have built a PIN-based mini-profiler to diagnose it, here's is an example of a diff it produces for a method context representative of the regression: PIN diff. It took me some time to realize this, but the cause is rather prosaic: LclVarDsc goes from 88 to 80 bytes and this causes MSVC to use lea + shl instead of imul for pointer arithmetic fetching LclVarDsc*s, inflating the retired instructions count. Evidently, this also pessimizes MSVC's inliner sometimes, but I am not too worried about that given we have native PGO.

To confirm, I added this "useless" padding to LclVarDsc:

    unsigned lvSlotNum; // original slot # (if remapped)

+   unsigned pad1;
+   unsigned pad2;

And the regression went away.

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

ghost commented Jul 3, 2022

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

Issue Details

Saves 0.7% in memory consumption when CG-ing CoreLib.

We're mostly not expecting diffs, except a few (< 10 cases across all of SPMI) where the new "is normed" logic in fgFindJumpTargets leads to us to inline something we didn't before. The reason this happens is that previously we would assign "is normed" false in case the inlining info had TI_ERROR (invalid) lclVerTypeInfos in it.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

@SingleAccretion
Copy link
Contributor Author

@dotnet/jit-contrib

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup.

@AndyAyersMS AndyAyersMS merged commit 2bcf0f7 into dotnet:main Jul 23, 2022
@SingleAccretion SingleAccretion deleted the Delete-lvVerTypeInfo branch July 23, 2022 16:51
@jakobbotsch
Copy link
Member

I have built a PIN-based mini-profiler to diagnose it, here's is an example of a diff it produces for a method context representative of the regression: PIN diff

This was really cool btw, may I suggest you make it available? :-)

@SingleAccretion
Copy link
Contributor Author

SingleAccretion commented Aug 6, 2022

may I suggest you make it available? :-)

Sure, it is a very simplistic single-threaded tool: https://gist.github.com/SingleAccretion/322071577c481040e409b98b6e936adf.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 5, 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.

3 participants