Skip to content

Conversation

@benaadams
Copy link
Member

@benaadams benaadams commented Jun 17, 2020

Currently Rosyln zeros the the AsyncMethodBuilder field with .locals init and then assigns the result of .Create to that field.

When .Create is inlined it is also zeros; so the assignment is a second set of zeroing.

The Jit has an optimization where it will remove the second set of zeroing dotnet/runtime#36918 ("Optimization to remove redundant zero initializations"); however this only kicks in if there aren't any preceding non-zero assignments.

e.g.

G_M51821_IG01:
       push     rsi
       sub      rsp, 144
       vzeroupper                                ; zero
       xor      rax, rax                         ;  |
       mov      qword ptr [rsp+28H], rax         ;  |
       vxorps   xmm4, xmm4                       ;  |
       mov      rax, -96                         ;  |
       vmovdqa  xmmword ptr [rsp+rax+90H], xmm4  ;  |
       vmovdqa  xmmword ptr [rsp+rax+A0H], xmm4  ;  |
       vmovdqa  xmmword ptr [rsp+rax+B0H], xmm4  ;  |
       add      rax, 48                          ;  |
       jne      SHORT  -5 instr                  ;  V
       mov      rsi, rdx
						;; bbWeight=1    PerfScore 8.58
G_M51821_IG02:
       mov      gword ptr [rsp+28H], r8      ; save param ** blocks redundant zero elimination **
       xor      ecx, ecx                     ; re-zero
       vxorps   xmm0, xmm0                   ;    |
       vmovdqu  xmmword ptr [rsp+38H], xmm0  ;    |
       vmovdqu  xmmword ptr [rsp+48H], xmm0  ;    |
       mov      qword ptr [rsp+58H], rcx     ;    V
       mov      dword ptr [rsp+30H], -1
       lea      rcx, [rsp+28H]
       call     System.Runtime.CompilerServices.AsyncMethodBuilderCore:Start(byref)
       lea      rcx, bword ptr [rsp+38H]
       mov      rdx, rsi
       call     System.Runtime.CompilerServices.AsyncValueTaskMethodBuilder`1[ReadResult][System.IO.Pipelines.ReadResult]:get_Task():System.Threading.Tasks.ValueTask`1[ReadResult]:this
       mov      rax, rsi

This PR moves the call to .Create() to the first step; prior to saving parameters to the fields so they do not block the redundant zero elimination optimization.

/cc @erozenfeld @stephentoub

From dotnet/runtime#2325 (comment)

Regarding your second example where we first assign to a gc field and then initialize the struct field: currently my optimization doesn't handle that. It's possible to extend it to handle that case but that would require keeping track of field initializations and may be a little tricky.

However, now the answer to your question

Or would changing it to move all the zeroing above that first assignment to non-zero (which would be a much more complex refactoring in Roslyn) help?

is yes.

If the struct field zero initialization was done before the first assignment, it would be eliminated by the new optimization.

Resolves #45295

@benaadams
Copy link
Member Author

Additional improvements have been added to Jit to take more advantage of this Rosyln change dotnet/runtime#38314

@jaredpar
Copy link
Member

Thanks for the update. I'm going to take a look at this later tonight.

@benaadams
Copy link
Member Author

It's quite a common pattern; especially since CancellationToken is normally one of the parameters even if there aren't any others

@jaredpar
Copy link
Member

@benaadams thanks this looks great!

Can u retarget this to release/dev16.8-preview1? At the moment "master" targets 16.7 and we're pretty late in the cycle there. Really only taking significant bug fixes.

@benaadams benaadams changed the base branch from master to release/dev16.8-preview1 June 27, 2020 15:01
@benaadams
Copy link
Member Author

Can u retarget this to release/dev16.8-preview1?

Done

@jaredpar jaredpar merged commit db87fbd into dotnet:release/dev16.8-preview1 Jun 29, 2020
@jaredpar
Copy link
Member

Thanks @benaadams!

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.

Order of initialization in async kickoff method with params blocks Jit zero init eliding

4 participants