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

[NativeAOT] Optimize multicast delegate thunk #104219

Closed
wants to merge 1 commit into from

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Jul 1, 2024

  • Use RawCalli
  • Use Unsafe cast of invocation list

These changes make the codegen of multicast delegate thunk to be more similar to CoreCLR w/ JIT.

- Use RawCalli
- Use Unsafe cast of invocation list

These changes make the codegen of multicast delegate thunk to be more
similar to CoreCLR w/ JIT.
Copy link
Contributor

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

@jkotas
Copy link
Member Author

jkotas commented Jul 1, 2024

Inspired by #104192

Baseline:

System.Action__InvokeMulticastThunk:
push    r14
push    rdi
push    rsi
push    rbp
push    rbx
sub     rsp,20h
mov     rbx,rcx
mov     rdx,qword ptr [rbx+10h]
mov     rsi,rdx
test    rsi,rsi
je      repro!System.Action__InvokeMulticastThunk+0x2d (00007ff7`5ffb66cd)
lea     rcx,[repro!??_7__Array<System.Delegate_Wrapper>@@6B@ (00007ff7`5ffed6d0)]
cmp     qword ptr [rsi],rcx
je      repro!System.Action__InvokeMulticastThunk+0x2d (00007ff7`5ffb66cd)
call    repro!System.Runtime.TypeCast__CheckCastAny (00007ff7`5ff97d20)
mov     rsi,rax
mov     ebx,dword ptr [rbx+18h]
mov     edi,dword ptr [rsi+8]
xor     ebp,ebp
cmp     ebp,edi
jae     repro!System.Action__InvokeMulticastThunk+0x70 (00007ff7`5ffb6710)
mov     rcx,qword ptr [rsi+rbp*8+10h]
mov     rax,qword ptr [rcx+8]
mov     r14,qword ptr [rcx+20h] ds:00000189`cb003a80={repro!System.Action__InvokeOpenStaticThunk (00007ff7`5ffb67e0)}
test    r14b,2
jne     repro!System.Action__InvokeMulticastThunk+0x54 (00007ff7`5ffb66f4)
mov     rcx,rax
call    r14
jmp     repro!System.Action__InvokeMulticastThunk+0x5f (00007ff7`5ffb66ff)
mov     rdx,qword ptr [r14+6]
mov     rcx,rax
call    qword ptr [r14-2]
inc     ebp
cmp     ebx,ebp
jne     repro!System.Action__InvokeMulticastThunk+0x35 (00007ff7`5ffb66d5)
add     rsp,20h
pop     rbx
pop     rbp
pop     rsi
pop     rdi
pop     r14
ret
call    repro!Internal.Runtime.CompilerHelpers.ThrowHelpers__ThrowIndexOutOfRangeException (00007ff7`5ffa1490)
int     3

PR:

S_P_CoreLib_System_Action__InvokeMulticastThunk:
push    rdi
push    rsi
push    rbp
push    rbx
sub     rsp,28h
mov     rbx,qword ptr [rcx+10h]
mov     esi,dword ptr [rcx+18h]
mov     edi,dword ptr [rbx+8]
xor     ebp,ebp
cmp     ebp,edi
jae     repro!S_P_CoreLib_System_Action__InvokeMulticastThunk+0x39 (00007ff6`1c5e3c99)
mov     rcx,qword ptr [rbx+rbp*8+10h]
mov     rax,qword ptr [rcx+8]
mov     rdx,qword ptr [rcx+20h]
mov     rcx,rax
call    rdx
inc     ebp
cmp     esi,ebp
jne     repro!S_P_CoreLib_System_Action__InvokeMulticastThunk+0x14 (00007ff6`1c5e3c74)
add     rsp,28h
pop     rbx
pop     rbp
pop     rsi
pop     rdi
ret
call    repro!S_P_CoreLib_Internal_Runtime_CompilerHelpers_ThrowHelpers__ThrowIndexOutOfRangeException (00007ff6`1c5cf250)
int     3
nop

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this pull request Jul 1, 2024
Alternative to dotnet#104219 for consideration.

RyuJIT generates somewhat better code for the canonical `Invoke` pattern:

The other PR:

```
00007FF7AE041978  mov         rcx,qword ptr [rbx+rbp*8+10h]
00007FF7AE04197D  mov         rax,qword ptr [rcx+8]
00007FF7AE041981  mov         rdx,qword ptr [rcx+20h]
00007FF7AE041985  mov         rcx,rax
00007FF7AE041988  call        rdx
```

This PR:

```
00007FF69D2B1978  mov         rax,qword ptr [rbx+rbp*8+10h]
00007FF69D2B197D  mov         rcx,qword ptr [rax+8]
00007FF69D2B1981  call        qword ptr [rax+20h]
```
@MichalStrehovsky
Copy link
Member

This looks good too, but I have a slightly different proposal for consideration at #104222.

@jkotas
Copy link
Member Author

jkotas commented Jul 1, 2024

Closing in favor of #104222.

@jkotas jkotas closed this Jul 1, 2024
@jkotas jkotas deleted the delegate-multicast branch July 1, 2024 07:02
MichalStrehovsky added a commit that referenced this pull request Jul 3, 2024
Alternative to #104219 for consideration.

RyuJIT generates somewhat better code for the canonical `Invoke` pattern:

The other PR:

```
00007FF7AE041978  mov         rcx,qword ptr [rbx+rbp*8+10h]
00007FF7AE04197D  mov         rax,qword ptr [rcx+8]
00007FF7AE041981  mov         rdx,qword ptr [rcx+20h]
00007FF7AE041985  mov         rcx,rax
00007FF7AE041988  call        rdx
```

This PR:

```
00007FF69D2B1978  mov         rax,qword ptr [rbx+rbp*8+10h]
00007FF69D2B197D  mov         rcx,qword ptr [rax+8]
00007FF69D2B1981  call        qword ptr [rax+20h]
```
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants