Skip to content

Better array pattern index codegen#38988

Merged
agocke merged 3 commits intodotnet:masterfrom
agocke:better-array-pattern-index-codegen
Oct 8, 2019
Merged

Better array pattern index codegen#38988
agocke merged 3 commits intodotnet:masterfrom
agocke:better-array-pattern-index-codegen

Conversation

@agocke
Copy link
Member

@agocke agocke commented Oct 1, 2019

Fixes #38942

@agocke agocke force-pushed the better-array-pattern-index-codegen branch from 60c1add to 8fbd05a Compare October 1, 2019 23:51
@agocke agocke marked this pull request as ready for review October 2, 2019 22:29
@agocke agocke requested a review from a team as a code owner October 2, 2019 22:29
Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

LGTM (commit 2)

{
// Code size 17 (0x11)
.maxstack 3
.locals init (int V_0)
Copy link
Member

Choose a reason for hiding this comment

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

any reason we capture this local?

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like it's only used for a conversion below in the stloc/ldloc pair. Probably unnecessary, but the JIT gets rid of that, so it's no different in performance. Not sure why codegen makes the local but it may be possible to improve.

@agocke agocke requested a review from a team October 7, 2019 17:40
@agocke agocke merged commit 5a24d13 into dotnet:master Oct 8, 2019
@agocke agocke deleted the better-array-pattern-index-codegen branch October 8, 2019 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Index indexing for arrays does not use pattern-based Index

5 participants