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.

Sign up for GitHub

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

Jump to bottom

Eliminate ToArray bounds checks #81001

Merged
merged 2 commits into from
Jan 23, 2023
Merged

Eliminate ToArray bounds checks #81001

merged 2 commits into from
Jan 23, 2023

Conversation

xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Jan 22, 2023 •
edited
Loading

See #80633 (comment):

// crossgen2 8.0.0-alpha.1.23061.99+71bb0d481086b8b8a89a99a17ec80a861f32dc5d

C:ToArray_Base():int[]:this:
; Emitting BLENDED_CODE for X64 CPU with SSE2 - Unix
       push     rbp
       push     rbx
       push     rax
       lea      rbp, [rsp+10H]
						;; size=8 bbWeight=1 PerfScore 3.50
       mov      eax, dword ptr [rdi+0CH]
       mov      ebx, dword ptr [rdi+08H]
       sub      eax, ebx
       movsxd   rdi, eax
       call     [CORINFO_HELP_READYTORUN_NEWARR_1]
       xor      edi, edi
       mov      esi, dword ptr [rax+08H]
       test     esi, esi
       je       SHORT G_M50617_IG04
						;; size=26 bbWeight=1 PerfScore 11.00
G_M50617_IG03:
       cmp      edi, esi
       jae      SHORT G_M50617_IG05
       mov      edx, edi
       mov      dword ptr [rax+4*rdx+10H], ebx
       inc      ebx
       inc      edi
       cmp      esi, edi
       jne      SHORT G_M50617_IG03
						;; size=18 bbWeight=4 PerfScore 17.00
G_M50617_IG04:
       add      rsp, 8
       pop      rbx
       pop      rbp
       ret      
						;; size=7 bbWeight=1 PerfScore 2.25
G_M50617_IG05:
       call     [CORINFO_HELP_RNGCHKFAIL]
       int3     
						;; size=7 bbWeight=0 PerfScore 0.00

C:ToArray_Diff():int[]:this:
; Emitting BLENDED_CODE for X64 CPU with SSE2 - Unix
       push     rbp
       push     rbx
       push     rax
       lea      rbp, [rsp+10H]
						;; size=8 bbWeight=1 PerfScore 3.50
       mov      eax, dword ptr [rdi+0CH]
       mov      ebx, dword ptr [rdi+08H]
       sub      eax, ebx
       movsxd   rdi, eax
       call     [CORINFO_HELP_READYTORUN_NEWARR_1]
       xor      edi, edi
       mov      esi, dword ptr [rax+08H]
       test     esi, esi
       jle      SHORT G_M19969_IG04
						;; size=26 bbWeight=1 PerfScore 11.00
G_M19969_IG03:
       mov      edx, edi
       mov      dword ptr [rax+4*rdx+10H], ebx
       inc      ebx
       inc      edi
       cmp      esi, edi
       jg       SHORT G_M19969_IG03
						;; size=14 bbWeight=4 PerfScore 12.00
G_M19969_IG04:
       add      rsp, 8
       pop      rbx
       pop      rbp
       ret      
						;; size=7 bbWeight=1 PerfScore 2.25

cc: @stephentoub

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jan 22, 2023
@ghost
Copy link

ghost commented Jan 22, 2023

Tagging subscribers to this area: @dotnet/area-system-linq
See info in area-owners.md if you want to be subscribed.

Issue Details

See #80633 (comment):

// crossgen2 8.0.0-alpha.1.23061.99+71bb0d481086b8b8a89a99a17ec80a861f32dc5d

C:ToArray_Base():int[]:this:
; Emitting BLENDED_CODE for X64 CPU with SSE2 - Unix
       push     rbp
       push     rbx
       push     rax
       lea      rbp, [rsp+10H]
						;; size=8 bbWeight=1 PerfScore 3.50
       mov      eax, dword ptr [rdi+0CH]
       mov      ebx, dword ptr [rdi+08H]
       sub      eax, ebx
       movsxd   rdi, eax
       call     [CORINFO_HELP_READYTORUN_NEWARR_1]
       xor      edi, edi
       mov      esi, dword ptr [rax+08H]
       test     esi, esi
       je       SHORT G_M50617_IG04
						;; size=26 bbWeight=1 PerfScore 11.00
G_M50617_IG03:
       cmp      edi, esi
       jae      SHORT G_M50617_IG05
       mov      edx, edi
       mov      dword ptr [rax+4*rdx+10H], ebx
       inc      ebx
       inc      edi
       cmp      esi, edi
       jne      SHORT G_M50617_IG03
						;; size=18 bbWeight=4 PerfScore 17.00
G_M50617_IG04:
       add      rsp, 8
       pop      rbx
       pop      rbp
       ret      
						;; size=7 bbWeight=1 PerfScore 2.25
G_M50617_IG05:
       call     [CORINFO_HELP_RNGCHKFAIL]
       int3     
						;; size=7 bbWeight=0 PerfScore 0.00

C:ToArray_Diff():int[]:this:
; Emitting BLENDED_CODE for X64 CPU with SSE2 - Unix
       push     rbp
       push     rbx
       push     rax
       lea      rbp, [rsp+10H]
						;; size=8 bbWeight=1 PerfScore 3.50
       mov      eax, dword ptr [rdi+0CH]
       mov      ebx, dword ptr [rdi+08H]
       sub      eax, ebx
       movsxd   rdi, eax
       call     [CORINFO_HELP_READYTORUN_NEWARR_1]
       xor      edi, edi
       mov      esi, dword ptr [rax+08H]
       test     esi, esi
       jle      SHORT G_M19969_IG04
						;; size=26 bbWeight=1 PerfScore 11.00
G_M19969_IG03:
       mov      edx, edi
       mov      dword ptr [rax+4*rdx+10H], ebx
       inc      ebx
       inc      edi
       cmp      esi, edi
       jg       SHORT G_M19969_IG03
						;; size=14 bbWeight=4 PerfScore 12.00
G_M19969_IG04:
       add      rsp, 8
       pop      rbx
       pop      rbp
       ret      
						;; size=7 bbWeight=1 PerfScore 2.25
Author: xtqqczze
Assignees: -
Labels:

area-System.Linq, community-contribution
Milestone: -

@xtqqczze xtqqczze changed the title Remove RangeIterator.ToArray bounds check Eliminate RangeIterator.ToArray bounds check Jan 22, 2023
@xtqqczze xtqqczze changed the title Eliminate RangeIterator.ToArray bounds check Eliminate RangeIterator.ToArray() bounds check Jan 22, 2023
@xtqqczze xtqqczze changed the title Eliminate RangeIterator.ToArray() bounds check Eliminate RangeIterator.ToArray bounds check Jan 22, 2023
stephentoub
stephentoub approved these changes Jan 22, 2023
View reviewed changes
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks

@xtqqczze xtqqczze changed the title Eliminate RangeIterator.ToArray bounds check Eliminate ToArray bounds checks Jan 22, 2023
@xtqqczze
Copy link
Contributor Author

@stephentoub Found some more instances of the same.

MichalPetryka
MichalPetryka reviewed Jan 22, 2023
View reviewed changes
src/libraries/System.Linq/src/System/Linq/Partition.SpeedOpt.cs
Comment on lines +257 to 260
for (int i = 0, curIdx = _minIndexInclusive; i < array.Length; ++i, ++curIdx)
{
array[i] = _source[curIdx];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (int i = 0, curIdx = _minIndexInclusive; i < array.Length; ++i, ++curIdx)
{
array[i] = _source[curIdx];
}
int minIndexInclusive = _minIndexInclusive;
for (int i = 0; i < array.Length; ++i)
{
array[i] = _source[i + minIndexInclusive];
}

This feels a bit simpler to me.

@stephentoub stephentoub merged commit a272954 into dotnet:main Jan 23, 2023
@xtqqczze xtqqczze mentioned this pull request Jan 23, 2023
Merged
mdh1418 pushed a commit to mdh1418/runtime that referenced this pull request Jan 24, 2023
@xtqqczze @mdh1418
Eliminate ToArray bounds checks (dotnet#81001)
77aeaac 
* Remove `RangeIterator.ToArray` bounds check

dotnet#80633 (comment)

* Eliminate additional `ToArray` bounds checks
@xtqqczze xtqqczze mentioned this pull request Jan 24, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Feb 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MichalPetryka MichalPetryka MichalPetryka left review comments

@stephentoub stephentoub stephentoub approved these changes

Assignees
No one assigned
Labels
area-System.Linq community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xtqqczze @stephentoub @MichalPetryka