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

RuntimeHelpers.CreateSpan optimization for stackalloc #57123

Merged
merged 6 commits into from
Oct 14, 2021

Conversation

svick
Copy link
Contributor

@svick svick commented Oct 13, 2021

This PR changes the IL generated for stackalloc in the following way:

  • If an existing optimization applies (loading byte arrays directly from PE data; using initblk when all bytes are the same), there is no change.
  • Otherwise, if it's converted to ReadOnlySpan<T> and all initializers are constant, the whole stackalloc is replaced by a call to CreateSpan.
  • Otherwise (e.g. when converted to Span<T> or when only some initializers are constant), CreateSpan is used as a part of regular stackalloc codegen, together with cpblk.

The IL for array initializers is also changed:

  • If an array initializer for a type larger than byte with all initializers constant is converted to ROS, the whole expression is replaced with CreateSpan.

There are still some questions I have and things that would need addressing to make this code production quality:

  1. Is it okay to call GetPinnableReference (without any pinning) to get the backing data for the result of CreateSpan, for use in cpblk?
  2. Alignment. As I understand it, the compiler currently does not align the metadata blocks containing initialization data in any way. If this is a requirement, that would need to change.
  3. ENC. This optimization does not work with ENC and the code needs to know whether to apply this optimization at the lowering stage. But I don't know how to find out if ENC is used from inside of LocalRewriter. Is there some way to do that?

@ghost ghost added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Oct 13, 2021
@svick svick force-pushed the initializespan-helper branch 3 times, most recently from 83489e2 to 609cba8 Compare October 13, 2021 20:35
@svick svick marked this pull request as ready for review October 14, 2021 14:57
@svick svick requested a review from a team as a code owner October 14, 2021 14:57
Copy link
Contributor

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

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

cool!

@cston cston merged commit 4f9e976 into dotnet:demos/lowlevelhackathon Oct 14, 2021
@cston
Copy link
Member

cston commented Oct 14, 2021

Thanks @svick!

@svick svick deleted the initializespan-helper branch October 14, 2021 19:04
@alrz
Copy link
Contributor

alrz commented Apr 23, 2022

lowlevelhackathon

So this is not supposed to reach main?

@svick
Copy link
Contributor Author

svick commented Apr 23, 2022

@alrz

So this is not supposed to reach main?

It is, at some point. As far as I can see, the required runtime code is ready (dotnet/runtime#60948 (comment)), so I think the next step would be to get answers to the questions I had above and then this could be probably merged into main.

I don't know if I'm going to be able to do that soon.

@jaredpar
Copy link
Member

It's on the list of features we're trying to get into C# 11. This feature is mostly in the "just need to find some keyboard time" vs. "needs a lot of design". That is why we've pushed it further back in the schedule as we've prioritized items that need more design work.

The schedule is very tight right now though as we had a number of last second asks that we're dealing with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee.
Projects
Development

Successfully merging this pull request may close these issues.

5 participants