Task/TaskAwaiter completion makes lots of duplicate memory reads

[benaadams]

public async Task CompletedTask()
{
    for (int i = 0; i < 100; i++)
        await Task.CompletedTask;
}

Inlines into 06000007 <CompletedTask>d__3:MoveNext():this
  [1 IL=0019 TR=000066 06001F0C] [below ALWAYS_INLINE size] Task:get_CompletedTask():ref
  [2 IL=0024 TR=000069 06001F2E] [below ALWAYS_INLINE size] Task:GetAwaiter():struct:this
    [3 IL=0001 TR=000172 060049ED] [below ALWAYS_INLINE size] TaskAwaiter:.ctor(ref):this
  [4 IL=0032 TR=000079 060049EE] [below ALWAYS_INLINE size] TaskAwaiter:get_IsCompleted():bool:this
    [5 IL=0006 TR=000194 06001F03] [below ALWAYS_INLINE size] Task:get_IsCompleted():bool:this
      [6 IL=0010 TR=000206 06001F04] [below ALWAYS_INLINE size] Task:IsCompletedMethod(int):bool
  [7 IL=0064 TR=000109 060048CB] [below ALWAYS_INLINE size] AsyncTaskMethodBuilder:AwaitUnsafeOnCompleted(byref,byref):this
    [0 IL=0008 TR=000227 060048D5] [FAILED: has exception handling] AsyncTaskMethodBuilder`1:AwaitUnsafeOnCompleted(byref,byref):this
  [8 IL=0101 TR=000044 060049F1] [below ALWAYS_INLINE size] TaskAwaiter:GetResult():this
    [9 IL=0006 TR=000239 060049F2] [below ALWAYS_INLINE size] TaskAwaiter:ValidateEnd(ref)
      [10 IL=0001 TR=000245 06001EE7] [aggressive inline attribute] Task:get_IsWaitNotificationEnabledOrNotRanToCompletion():bool:this
      [11 IL=0009 TR=000255 060049F3] [profitable inline] TaskAwaiter:HandleNonSuccessAndDebuggerNotification(ref)
        [12 IL=0001 TR=000272 06001F03] [below ALWAYS_INLINE size] Task:get_IsCompleted():bool:this
          [13 IL=0010 TR=000327 06001F04] [below ALWAYS_INLINE size] Task:IsCompletedMethod(int):bool
        [14 IL=0019 TR=000310 06001F39] [below ALWAYS_INLINE size] Task:InternalWait(int,struct):bool:this
        [0 IL=0026 TR=000281 06001EE5] [FAILED: unprofitable inline] Task:NotifyDebuggerOfWaitCompletionIfNecessary():bool:this
        [15 IL=0033 TR=000288 06001F05] [profitable inline] Task:get_IsCompletedSuccessfully():bool:this
        [0 IL=0041 TR=000298 060049F4] [FAILED: noinline per IL/cached result] TaskAwaiter:ThrowForNonSuccess(ref)
  [0 IL=0150 TR=000151 060048CE] [FAILED: within catch region] AsyncTaskMethodBuilder:SetException(ref):this
  [16 IL=0171 TR=000126 060048CD] [profitable inline] AsyncTaskMethodBuilder:SetResult():this
    [17 IL=0011 TR=000368 060048DE] [profitable inline] AsyncTaskMethodBuilder`1:SetResult(ref):this
      [0 IL=0026 TR=000390 060048DC] [FAILED: noinline per IL/cached result] AsyncTaskMethodBuilder`1:SetExistingTaskResult(struct):this

;  V00 this         [V00,T01] ( 22, 40   )   byref  ->  [rbp+0x10]   do-not-enreg[H] this
;  V01 loc0         [V01,T12] (  2,  2   )     int  ->  rcx        
;  V02 loc1         [V02    ] (  6, 19   )  struct ( 8) [rbp-0x10]   do-not-enreg[XS] must-init addr-exposed ld-addr-op
;  V03 loc2         [V03,T04] (  2, 16   )     int  ->  rdx        
;* V04 loc3         [V04    ] (  0,  0   )     ref  ->  zero-ref    class-hnd
;  V05 OutArgs      [V05    ] (  1,  1   )  lclBlk (32) [rsp+0x00]   "OutgoingArgSpace"
;* V06 tmp1         [V06    ] (  0,  0   )     int  ->  zero-ref    "dup spill"
;  V07 tmp2         [V07,T15] (  2,  0   )     ref  ->  rdx         class-hnd "impSpillSpecialSideEff"
;  V08 tmp3         [V08,T02] (  3, 24   )     ref  ->  rdx         class-hnd "Inlining Arg"
;* V09 tmp4         [V09    ] (  0,  0   )  struct ( 8) zero-ref    "NewObj constructor temp"
;  V10 tmp5         [V10,T03] (  2, 16   )     ref  ->  rdx         class-hnd "Inlining Arg"
;  V11 tmp6         [V11,T06] (  2,  8   )     int  ->  rdx         "Inline stloc first use temp"
;  V12 tmp7         [V12,T07] (  2,  4   )   byref  ->  rcx         "Inlining Arg"
;  V13 tmp8         [V13,T00] (  7, 44   )     ref  ->  rsi         class-hnd "Inlining Arg"
;* V14 tmp9         [V14    ] (  0,  0   )  struct ( 8) zero-ref    ld-addr-op "Inline ldloca(s) first use temp"
;* V15 tmp10        [V15    ] (  0,  0   )    bool  ->  zero-ref    "Inline stloc first use temp"
;  V16 tmp11        [V16,T10] (  2,  4   )     int  ->  rcx         "Inline stloc first use temp"
;* V17 tmp12        [V17    ] (  0,  0   )  struct ( 8) zero-ref    "Inlining Arg"
;  V18 tmp13        [V18,T08] (  2,  4   )   byref  ->  rcx         "Inlining Arg"
;  V19 tmp14        [V19,T05] (  4,  8   )   byref  ->  rcx         "Inlining Arg"
;  V20 tmp15        [V20,T13] (  2,  2   )  struct ( 8) [rbp-0x18]   do-not-enreg[SF] must-init ld-addr-op "Inline ldloca(s) first use temp"
;  V21 tmp16        [V21,T09] (  2,  4   )     ref  ->  rdx         class-hnd exact "Inlining Arg"
;  V22 tmp17        [V22    ] (  6, 19   )     ref  ->  [rbp-0x10]   do-not-enreg[X] addr-exposed V02.m_task(offs=0x00) P-DEP "field V02.m_task (fldOffset=0x0)"
;* V23 tmp18        [V23    ] (  0,  0   )     ref  ->  zero-ref    V09.m_task(offs=0x00) P-INDEP "field V09.m_task (fldOffset=0x0)"
;* V24 tmp19        [V24,T11] (  0,  0   )     ref  ->  zero-ref    V14._source(offs=0x00) P-INDEP "field V14._source (fldOffset=0x0)"
;* V25 tmp20        [V25,T14] (  0,  0   )     ref  ->  zero-ref    V17._source(offs=0x00) P-INDEP "field V17._source (fldOffset=0x0)"
;  V26 PSPSym       [V26    ] (  1,  1   )    long  ->  [rbp-0x20]   do-not-enreg[X] addr-exposed "PSPSym"
;* V27 tmp22        [V27    ] (  0,  0   )   byref  ->  zero-ref    "optAddCopies"
G_M10160_IG01:
       55                   push     rbp
       56                   push     rsi
       4883EC38             sub      rsp, 56
       488D6C2440           lea      rbp, [rsp+40H]
       33C0                 xor      rax, rax
       488945F0             mov      qword ptr [rbp-10H], rax
       488945E8             mov      qword ptr [rbp-18H], rax
       488965E0             mov      qword ptr [rbp-20H], rsp
       48894D10             mov      bword ptr [rbp+10H], rcx   ; set [rbp+10H] from rcx

G_M10160_IG02:
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx
       8B09                 mov      ecx, dword ptr [rcx]

G_M10160_IG03:
       85C9                 test     ecx, ecx
       0F8486000000         je       G_M10160_IG05
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx  
       33D2                 xor      edx, edx
       895104               mov      dword ptr [rcx+4], edx
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx  
       83790464             cmp      dword ptr [rcx+4], 100
       0F8DFF000000         jge      G_M10160_IG09

G_M10160_IG04:                                              ; ** for body start
       48B9783EECCFFB7F0000 mov      rcx, 0x7FFBCFEC3E78
       BA40020000           mov      edx, 576
       E81AACBA5F           call     CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE
       48BAF8159DDC14020000 mov      rdx, 0x214DC9D15F8
       488B12               mov      rdx, gword ptr [rdx]
       8B0A                 mov      ecx, dword ptr [rdx]
       488955F0             mov      gword ptr [rbp-10H], rdx
       488B55F0             mov      rdx, gword ptr [rbp-10H]
       8B5234               mov      edx, dword ptr [rdx+52]
       F7C200006001         test     edx, 0x1600000
       755A                 jne      SHORT G_M10160_IG06
       488B5510             mov      rdx, bword ptr [rbp+10H]    ; read [rbp+10H] to rdx  
       33C9                 xor      ecx, ecx
       890A                 mov      dword ptr [rdx], ecx
       488B5510             mov      rdx, bword ptr [rbp+10H]    ; read [rbp+10H] to rdx    
       488D4A10             lea      rcx, bword ptr [rdx+16]
       488B55F0             mov      rdx, gword ptr [rbp-10H]
       E8EF93BA5F           call     CORINFO_HELP_CHECKED_ASSIGN_REF
       488B5510             mov      rdx, bword ptr [rbp+10H]    ; read [rbp+10H] to rdx   
       3912                 cmp      dword ptr [rdx], edx
       488B5510             mov      rdx, bword ptr [rbp+10H]    ; read [rbp+10H] to rdx  
       488D4A08             lea      rcx, bword ptr [rdx+8]
       488D55F0             lea      rdx, bword ptr [rbp-10H]
       4C8B4510             mov      r8, bword ptr [rbp+10H]
       E8D4F5FFFF           call     AsyncTaskMethodBuilder`1:AwaitUnsafeOnCompleted(byref,byref):this
       E9D0000000           jmp      G_M10160_IG11

G_M10160_IG05:
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx  
       488B4910             mov      rcx, gword ptr [rcx+16]
       48894DF0             mov      gword ptr [rbp-10H], rcx
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx  
       4533C0               xor      r8, r8
       4C894110             mov      qword ptr [rcx+16], r8
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx 
       C701FFFFFFFF         mov      dword ptr [rcx], -1

G_M10160_IG06:
       488B75F0             mov      rsi, gword ptr [rbp-10H]
       8B4E34               mov      ecx, dword ptr [rsi+52]
       81E100000011         and      ecx, 0x11000000
       81F900000001         cmp      ecx, 0x1000000
       743C                 je       SHORT G_M10160_IG08
       8B4E34               mov      ecx, dword ptr [rsi+52]
       F7C100006001         test     ecx, 0x1600000
       7510                 jne      SHORT G_M10160_IG07
       488BCE               mov      rcx, rsi
       4533C0               xor      r8, r8
       BAFFFFFFFF           mov      edx, -1
       E83E27AC5A           call     Task:InternalWaitCore(int,struct):bool:this

G_M10160_IG07:
       488BCE               mov      rcx, rsi
       E8B600AC5A           call     Task:NotifyDebuggerOfWaitCompletionIfNecessary():bool:this
       8B4E34               mov      ecx, dword ptr [rsi+52]
       81E100006001         and      ecx, 0x1600000
       81F900000001         cmp      ecx, 0x1000000
       7408                 je       SHORT G_M10160_IG08
       488BCE               mov      rcx, rsi
       E8DDEBB05A           call     TaskAwaiter:ThrowForNonSuccess(ref)

G_M10160_IG08:
       488B5510             mov      rdx, bword ptr [rbp+10H]    ; read [rbp+10H] to rdx  
       8B5204               mov      edx, dword ptr [rdx+4]
       FFC2                 inc      edx
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx  
       895104               mov      dword ptr [rcx+4], edx
       488B5510             mov      rdx, bword ptr [rbp+10H]    ; read [rbp+10H] to rdx  
       837A0464             cmp      dword ptr [rdx+4], 100
       0F8C01FFFFFF         jl       G_M10160_IG04          ; ** for body end

G_M10160_IG09:
       488B5510             mov      rdx, bword ptr [rbp+10H]    ; read [rbp+10H] to rdx  
       C702FEFFFFFF         mov      dword ptr [rdx], -2
       488B5510             mov      rdx, bword ptr [rbp+10H]    ; read [rbp+10H] to rdx  
       3912                 cmp      dword ptr [rdx], edx
       488B5510             mov      rdx, bword ptr [rbp+10H]    ; read [rbp+10H] to rdx  
       488D4A08             lea      rcx, bword ptr [rdx+8]
       48BA88259DDC14020000 mov      rdx, 0x214DC9D2588
       488B12               mov      rdx, gword ptr [rdx]
       48833900             cmp      gword ptr [rcx], 0
       7507                 jne      SHORT G_M10160_IG10
       E80F93BA5F           call     CORINFO_HELP_CHECKED_ASSIGN_REF
       EB0E                 jmp      SHORT G_M10160_IG11

G_M10160_IG10:
       C645E800             mov      byte  ptr [rbp-18H], 0
       480FBE55E8           movsx    rdx, byte  ptr [rbp-18H]
       E8FFCCB05A           call     AsyncTaskMethodBuilder`1:SetExistingTaskResult(struct):this

G_M10160_IG11:
       90                   nop      

G_M10160_IG12:
       488D65F8             lea      rsp, [rbp-08H]
       5E                   pop      rsi
       5D                   pop      rbp
       C3                   ret      

G_M10160_IG13:
       55                   push     rbp
       56                   push     rsi
       4883EC28             sub      rsp, 40
       488B6920             mov      rbp, qword ptr [rcx+32]
       48896C2420           mov      qword ptr [rsp+20H], rbp
       488D6D40             lea      rbp, [rbp+40H]

G_M10160_IG14:
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx  
       C701FEFFFFFF         mov      dword ptr [rcx], -2
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx  
       3909                 cmp      dword ptr [rcx], ecx
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx  
       4883C108             add      rcx, 8
       E8A716B35A           call     AsyncTaskMethodBuilder:SetException(ref):this
       488D05C1FFFFFF       lea      rax, G_M10160_IG11

G_M10160_IG15:
       4883C428             add      rsp, 40
       5E                   pop      rsi
       5D                   pop      rbp
       C3                   ret      

; Total bytes of code 455, prolog size 29 for method <CompletedTask>d__3:MoveNext():this      

Can this be elided where its not changing the register (as its just set the line or so before)

/cc @stephentoub @AndyAyersMS

category:cq
theme:cse
skill-level:expert
cost:extra-large
impact:large

[benaadams]

Related dotnet/coreclr#21959

[AndyAyersMS]

This is similar in spirit to #6913 (addressed via dotnet/coreclr#9169) but the twist here is that there is (usually, not always) a write in between the apparently redundant reads. And the jit doesn't have the kind of alias analysis it would need to prove that the write can't modify the location just read...

In a few cases there is no write .. eg

       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx  
       3909                 cmp      dword ptr [rcx], ecx
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx

and it seems possible this could be optimized by the jit without needing to enhance its analysis abilities.

[mikedn]

Hmm, isn't this actually caused by exception handling? See lclvar info:

V00 this [V00,T01] ( 22, 40 ) byref -> [rbp+0x10] do-not-enreg[H] this

[benaadams]

I assume its related to

byref  ->  [rbp+0x10]   do-not-enreg[H] this

I tried to introduce some locals to break the dependency on the do-not-enreg; however they didn't seem to have any effect.

The TaskAwaiter:ValidateEnd(ref) inline-chain is based on a param static void ValidateEnd(Task task)

[9 IL=0006 TR=000239 060049F2] [below ALWAYS_INLINE size] TaskAwaiter:ValidateEnd(ref)
      [10 IL=0001 TR=000245 06001EE7] [aggressive inline attribute] Task:get_IsWaitNotificationEnabledOrNotRanToCompletion():bool:this
      [11 IL=0009 TR=000255 060049F3] [profitable inline] TaskAwaiter:HandleNonSuccessAndDebuggerNotification(ref)
        [12 IL=0001 TR=000272 06001F03] [below ALWAYS_INLINE size] Task:get_IsCompleted():bool:this
          [13 IL=0010 TR=000327 06001F04] [below ALWAYS_INLINE size] Task:IsCompletedMethod(int):bool
        [14 IL=0019 TR=000310 06001F39] [below ALWAYS_INLINE size] Task:InternalWait(int,struct):bool:this
        [0 IL=0026 TR=000281 06001EE5] [FAILED: unprofitable inline] Task:NotifyDebuggerOfWaitCompletionIfNecessary():bool:this
        [15 IL=0033 TR=000288 06001F05] [profitable inline] Task:get_IsCompletedSuccessfully():bool:this
        [0 IL=0041 TR=000298 060049F4] [FAILED: noinline per IL/cached result] TaskAwaiter:ThrowForNonSuccess(ref)

However when it gets inlined does the Jit recognise its all the same variable so merge them together; but unfortunately that's a do-not-enreg? (Might be completely wrong here 😄 )

[AndyAyersMS]

I wouldn't think EH would block the read-read cases though... will have to see what VNs actually get assigned.

[mikedn]

They all get the same VN ($80):

     (  5,  4) [000095] ------------              *  STMT      void  (IL   ???...0x02B)
N003 (  1,  1) [000088] ------------              |  /--*  CNS_INT   int    0 $40
N004 (  5,  4) [000094] -A-XG-------              \--*  ASG       int    $VN.Void
N002 (  3,  2) [000093] *--XG--N----                 \--*  IND       int    $40
N001 (  1,  1) [000087] ------------                    \--*  LCL_VAR   byref  V00 this         u:1 $80

***** BB05, stmt 14
     (  8,  7) [000102] ------------              *  STMT      void  (IL 0x030...0x032)
N007 (  3,  2) [000310] ----G--N----              |  /--*  LCL_VAR   ref   (AX) V15 tmp10         $2c6
N008 (  8,  7) [000311] -A-XG-------              \--*  ASG       ref    $VN.Void
N006 (  4,  4) [000309] *--X---N----                 \--*  IND       ref    $2c6
N004 (  1,  1) [000307] ------------                    |  /--*  CNS_INT   long   0 Fseq[m_task] $241
N005 (  4,  4) [000308] -------N----                    \--*  ADD       byref  $288
N002 (  1,  1) [000306] ------------                       |  /--*  CNS_INT   long   16 field offset Fseq[<>u__1] $240
N003 (  3,  3) [000304] -------N----                       \--*  ADD       byref  $280
N001 (  1,  1) [000305] ------------                          \--*  LCL_VAR   byref  V00 this         u:1 $80

***** BB05, stmt 15
     (  5,  5) [000229] ------------              *  STMT      void  (IL 0x037...  ???)
N004 (  1,  1) [000315] ------------              |        /--*  CNS_INT   long   8 field offset Fseq[<>t__builder] $243
N005 (  3,  3) [000316] ------------              |     /--*  ADD       byref  $282
N003 (  1,  1) [000314] ------------              |     |  \--*  LCL_VAR   byref  V00 this         u:1 $80
N006 (  5,  5) [000317] ---XG--N----              |  /--*  COMMA     byref  $283
N002 (  2,  2) [000313] ---X---N----              |  |  \--*  NULLCHECK byte   $1c7
N001 (  1,  1) [000312] ------------              |  |     \--*  LCL_VAR   byref  V00 this         u:1 $80
N008 (  5,  5) [000228] -A-XG---R---              \--*  ASG       byref  $283
N007 (  1,  1) [000227] D------N----                 \--*  LCL_VAR   byref  V12 tmp7         d:2 $282

***** BB05, stmt 16
     ( 19, 14) [000226] ------------              *  STMT      void  (IL 0x037...  ???)
N013 ( 19, 14) [000223] --CXG-------              \--*  CALL      void   System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[VoidTaskResult][System.Threading.Tasks.VoidTaskResult].AwaitUnsafeOnCompleted $VN.Void
N007 (  3,  3) [000221] L----------- arg1 in rdx     +--*  ADDR      long   $3c7
N006 (  3,  2) [000222] ----G--N----                 |  \--*  LCL_VAR   struct(AX)(P) V02 loc1         
                                                     |  \--*    ref    V02.m_task (offs=0x00) -> V15 tmp10         $541
N008 (  1,  1) [000218] ------------ this in rcx     +--*  LCL_VAR   byref  V12 tmp7         u:2 (last use) $282
N009 (  1,  1) [000108] ------------ arg2 in r8      \--*  LCL_VAR   byref  V00 this         u:1 $80

[AndyAyersMS]

Taking a closer look -- seems like CSE is not willing to consider locals even if they're somewhat costly...

https://github.com/dotnet/coreclr/blob/8ede2d4f5d85e4069e96046baf4aaf567172a87f/src/jit/optcse.cpp#L2624-L2625

This seems perhaps overly conservative -- we ought to be able to CSE unaliased locals (provided we don't cse across exception flow). Most of the time there's no apparent benefit to this, but if the local can't be enregistered then perhaps there is....

[benaadams]

In a few cases there is no write .. eg

       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx  
       3909                 cmp      dword ptr [rcx], ecx
       488B4D10             mov      rcx, bword ptr [rbp+10H]    ; read [rbp+10H] to rcx

Is doesn't seem to do anything with the result of the cmp; is this a null ref check?

[AndyAyersMS]

Yes, it's a null check.

[AndyAyersMS]

We don't compute live in/out of handler status until after we've computed costs. So CSE is using a stale cost estimate and decides that local is not a good CSE candidate. Not sure if it's feasible to revise the costing.

Am curious why assertion prop is not getting rid of those null checks. So I'll look into that first.

[AndyAyersMS]

Seems like lack of assertion prop might be easy to fix. Testing something now.