Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Add RuntimeHelpers.IsReferenceOrContainsReferences<T>()#9541

Merged
jkotas merged 1 commit into
dotnet:masterfrom
jkotas:IsReferenceOrContainsReferences
Feb 13, 2017
Merged

Add RuntimeHelpers.IsReferenceOrContainsReferences<T>()#9541
jkotas merged 1 commit into
dotnet:masterfrom
jkotas:IsReferenceOrContainsReferences

Conversation

@jkotas
Copy link
Copy Markdown
Member

@jkotas jkotas commented Feb 12, 2017

Rename JitHelpers.ContainsReferences() to RuntimeHelpers.IsReferenceOrContainsReferences() and make it public.

Work towards https://github.com/dotnet/corefx/issues/14047

@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Feb 12, 2017

cc @benaadams

I will fix this up if your change gets in first.

Rename JitHelpers.ContainsReferences<T>() to RuntimeHelpers.IsReferenceOrContainsReferences<T>() and make it public.

Work towards https://github.com/dotnet/corefx/issues/14047
@jkotas jkotas force-pushed the IsReferenceOrContainsReferences branch from a44545d to 3b8e6b1 Compare February 12, 2017 22:00
@benaadams
Copy link
Copy Markdown
Member

benaadams commented Feb 13, 2017

is asm emitted for this for reference types (in current incarnation)? from #9540 (comment)

G_M58440_IG01:
       56                   push     rsi
       4883EC30             sub      rsp, 48
       48894C2428           mov      qword ptr [rsp+28H], rcx    ; putting something stack 40
       488BF1               mov      rsi, rcx                    ; putting it in rsi

G_M58440_IG02:
       488B0E               mov      rcx, qword ptr [rsi]        ;** ?? --- redundant load?        
       448B4618             mov      r8d, dword ptr [rsi+24]     ; int size = _size;
       33C9                 xor      ecx, ecx                    ;** 0 ----^ clears earlier move
       894E18               mov      dword ptr [rsi+24], ecx     ; _size = 0;
       FF461C               inc      dword ptr [rsi+28]          ; _version++;
       4585C0               test     r8d, r8d                    ; if (size > 0)
       7E15                 jle      SHORT G_M58440_IG04         ; ---> jmp  size == 0
       488B4E08             mov      rcx, gword ptr [rsi+8]      ; _items
       33D2                 xor      edx, edx                    ; 0
       488D0500000000       lea      rax, [(reloc)]              ; Array.Clear;

G_M58440_IG03:
       4883C430             add      rsp, 48
       5E                   pop      rsi
       48FFE0               rex.jmp  rax                         ; Array.Clear(_items, 0, size);

G_M58440_IG04:
       4883C430             add      rsp, 48
       5E                   pop      rsi
       C3                   ret      

There seems to be extra unnecessary register stuff going on; which could also be entirely unrelated. Also I'm not entirely fluent in asm or ABI so I might be wrong.

Was trying to work out if the startup part could be reduced any; but not sure how it relates to the code

@jkotas jkotas merged commit 7b66079 into dotnet:master Feb 13, 2017
@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Feb 13, 2017

The redundant load is unused left over temp - opened #9547 on it. The unnecessary frame maybe side-effect of the unused left over temp.

@jkotas jkotas deleted the IsReferenceOrContainsReferences branch February 13, 2017 22:04
jorive pushed a commit to guhuro/coreclr that referenced this pull request May 4, 2017
Rename JitHelpers.ContainsReferences<T>() to RuntimeHelpers.IsReferenceOrContainsReferences<T>() and make it public.

Work towards https://github.com/dotnet/corefx/issues/14047
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…lr#9541)

Rename JitHelpers.ContainsReferences<T>() to RuntimeHelpers.IsReferenceOrContainsReferences<T>() and make it public.

Work towards https://github.com/dotnet/corefx/issues/14047

Commit migrated from dotnet/coreclr@7b66079
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants