Flag does_not_return early #7580

benaadams · 2016-10-12T08:18:59Z

All ThrowHelper calls are now recognised as cold code.

Adds gtCallMoreFlags |= GTF_CALL_M_DOES_NOT_RETURN to fgFindJumpTargets which happens in early in pipeline (which may mark the method as uninlinable, so prevent the later setting in fgFindBasicBlocks).

Count the BBJ_RETURNs in fgMakeBasicBlocks rather than recounting afterwards (perf)

Reduced the immediate throw path in ThrowHelper to help with recognition of the only throws condition.

There were some regressions with so much more recognised as cold code and moved out of the main execution path; so I added some common exception arguments as extra functions for the generics to call in ThrowHelper which resolved this. (Kept as individual commits)

With current branch -0.37 % diff improvement

Resolves https://github.com/dotnet/coreclr/issues/6744
Replacement for #6742

/cc @mikedn @AndyAyersMS @pgavlin

benaadams · 2016-10-12T08:22:19Z

jit-diff

Found files with textual diffs.

Summary:
(Note: Lower is better)

Total bytes of diff: -11727 (-0.37 % of base)
    diff is an improvement.

Total byte diff includes 1328 bytes from reconciling methods
        Base had    1 unique methods,       41 unique bytes
        Diff had   19 unique methods,     1369 unique bytes

Top file improvements by size (bytes):
      -11727 : System.Private.CoreLib.dasm (-0.37 % of base)

1 total files with size differences.

Top method regessions by size (bytes):
         181 : System.Private.CoreLib.dasm - ThrowHelper:GetArgumentOutOfRangeException(int,int,int):ref
         173 : System.Private.CoreLib.dasm - Dictionary`2:OnDeserialization(ref):this (29 methods)
         126 : System.Private.CoreLib.dasm - Comparer`1:System.Collections.IComparer.Compare(ref,ref):int:this (26 methods)
         125 : System.Private.CoreLib.dasm - ThrowHelper:GetWrongKeyTypeArgumentException(ref,ref):ref
         125 : System.Private.CoreLib.dasm - ThrowHelper:GetWrongValueTypeArgumentException(ref,ref):ref
         122 : System.Private.CoreLib.dasm - ThrowHelper:GetArgumentOutOfRangeException(int,int):ref
         112 : System.Private.CoreLib.dasm - ThrowHelper:GetArgumentException(int,int):ref
          99 : System.Private.CoreLib.dasm - Dictionary`2:System.Collections.ICollection.CopyTo(ref,int):this (29 methods)
          95 : System.Private.CoreLib.dasm - ThrowHelper:GetAddingDuplicateWithKeyArgumentException(ref):ref
          83 : System.Private.CoreLib.dasm - ThrowHelper:GetArgumentException(int):ref
          83 : System.Private.CoreLib.dasm - ThrowHelper:GetInvalidOperationException(int):ref
          81 : System.Private.CoreLib.dasm - KeyCollection:System.Collections.ICollection.CopyTo(ref,int):this (30 methods)
          80 : System.Private.CoreLib.dasm - ValueCollection:System.Collections.ICollection.CopyTo(ref,int):this (30 methods)
          69 : System.Private.CoreLib.dasm - List`1:GetRange(int,int):ref:this (23 methods)
          64 : System.Private.CoreLib.dasm - List`1:set_Capacity(int):this (26 methods)
          55 : System.Private.CoreLib.dasm - List`1:Insert(int,struct):this (11 methods)
          50 : System.Private.CoreLib.dasm - ThrowHelper:ThrowArgumentException_Argument_InvalidArrayType()
          50 : System.Private.CoreLib.dasm - ThrowHelper:ThrowInvalidOperationException_InvalidOperation_EnumNotStarted()
          50 : System.Private.CoreLib.dasm - ThrowHelper:ThrowInvalidOperationException_InvalidOperation_EnumEnded()
          50 : System.Private.CoreLib.dasm - ThrowHelper:ThrowInvalidOperationException_InvalidOperation_EnumFailedVersion()

Top method improvements by size (bytes):
       -1588 : System.Private.CoreLib.dasm - Enumerator:MoveNext():bool:this (113 methods)
        -945 : System.Private.CoreLib.dasm - EventProvider:WriteEvent(byref,long,long,ref):bool:this
        -904 : System.Private.CoreLib.dasm - Enumerator:System.Collections.IEnumerator.Reset():this (113 methods)
        -644 : System.Private.CoreLib.dasm - List`1:CopyTo(int,ref,int,int):this (23 methods)
        -598 : System.Private.CoreLib.dasm - List`1:FindLastIndex(int,int,ref):int:this (23 methods)
        -595 : System.Private.CoreLib.dasm - Enumerator:System.Collections.IEnumerator.get_Current():ref:this (113 methods)
        -331 : System.Private.CoreLib.dasm - List`1:Sort(int,int,ref):this (23 methods)
        -323 : System.Private.CoreLib.dasm - Enumerator:System.Collections.IDictionaryEnumerator.get_Key():ref:this (29 methods)
        -317 : System.Private.CoreLib.dasm - Enumerator:System.Collections.IDictionaryEnumerator.get_Value():ref:this (29 methods)
        -279 : System.Private.CoreLib.dasm - List`1:LastIndexOf(struct,int):int:this (9 methods)
        -246 : System.Private.CoreLib.dasm - Enumerator:System.Collections.IDictionaryEnumerator.get_Entry():struct:this (29 methods)
        -240 : System.Private.CoreLib.dasm - List`1:IndexOf(struct,int,int):int:this (9 methods)
        -234 : System.Private.CoreLib.dasm - List`1:RemoveAt(int):this (26 methods)
        -234 : System.Private.CoreLib.dasm - ReadOnlyCollection`1:System.Collections.IList.Add(ref):int:this (26 methods)
        -223 : System.Private.CoreLib.dasm - List`1:IndexOf(struct,int):int:this (9 methods)
        -208 : System.Private.CoreLib.dasm - Enumerator:MoveNextRare():bool:this (26 methods)
        -190 : System.Private.CoreLib.dasm - List`1:BinarySearch(int,int,struct,ref):int:this (9 methods)
        -184 : System.Private.CoreLib.dasm - List`1:FindIndex(int,int,ref):int:this (23 methods)
        -178 : System.Private.CoreLib.dasm - Array:LastIndexOf(ref,struct,int,int):int (9 methods)
        -171 : System.Private.CoreLib.dasm - List`1:Reverse(int,int):this (23 methods)

660 total methods with size differences.

benaadams · 2016-10-12T08:38:29Z

Array:Sort(ref,int,int,ref) asm (example of improved cold code recognition)

Pre

; Assembly listing for method Array:Sort(ref,int,int,ref)
; Emitting BLENDED_CODE for X64 CPU with SSE2
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 arg0         [V00,T02] (  6,   5  )     ref  ->  rbx        
;  V01 arg1         [V01,T01] (  7,   5.5)     int  ->  rdi        
;  V02 arg2         [V02,T00] (  7,   6  )     int  ->  rsi        
;  V03 arg3         [V03,T03] (  5,   3.5)     ref  ->  rbp        
;  V04 tmp0         [V04,T04] (  2,   2  )     ref  ->  rcx        
;  V05 OutArgs      [V05    ] (  1,   1  )  lclBlk (40) [rsp+0x00]  
;
; Lcl frame size = 40
G_M42325_IG01:
       push     rdi
       push     rsi
       push     rbp
       push     rbx
       sub      rsp, 40
       mov      rbx, rcx
       mov      edi, edx
       mov      esi, r8d
       mov      rbp, r9
G_M42325_IG02:
       test     rbx, rbx
       je       G_M42325_IG12
G_M42325_IG03:
       test     edi, edi
       jge      SHORT G_M42325_IG04
       call     ThrowHelper:ThrowIndexArgumentOutOfRange_NeedNonNegNumException()
G_M42325_IG04:
       test     esi, esi
       jge      SHORT G_M42325_IG05
       mov      ecx, 29
       mov      edx, 4
       call     ThrowHelper:ThrowArgumentOutOfRangeException(int,int)
G_M42325_IG05:
       mov      ecx, dword ptr [rbx+8]
       sub      ecx, edi
       cmp      ecx, esi
       jge      SHORT G_M42325_IG06
       mov      ecx, 23
       call     ThrowHelper:ThrowArgumentException(int)
G_M42325_IG06:
       cmp      esi, 1
       jle      SHORT G_M42325_IG10
       test     rbp, rbp
       je       SHORT G_M42325_IG07
       mov      rcx, qword ptr [(reloc)]
       mov      edx, 103
       call     CORINFO_HELP_GETSHARED_GCSTATIC_BASE_DYNAMICCLASS
       cmp      gword ptr [rax], rbp
       jne      SHORT G_M42325_IG09
G_M42325_IG07:
       lea      r9d, [rdi+rsi-1]
       mov      rcx, rbx
       mov      r8d, edi
       xor      rdx, rdx
       call     Array:TrySZSort(ref,ref,int,int):bool
       test     eax, eax
       je       SHORT G_M42325_IG09
G_M42325_IG08:
       add      rsp, 40
       pop      rbx
       pop      rbp
       pop      rsi
       pop      rdi
       ret      
G_M42325_IG09:
       call     ArraySortHelper`1:get_Default():ref
       mov      rcx, rax
       mov      gword ptr [rsp+20H], rbp
       mov      rdx, rbx
       mov      r8d, edi
       mov      r9d, esi
       lea      r11, [(reloc)]
       cmp      dword ptr [rcx], ecx
       call     qword ptr [r11]IArraySortHelper`1:Sort(ref,int,int,ref):this
G_M42325_IG10:
       nop      
G_M42325_IG11:
       add      rsp, 40
       pop      rbx
       pop      rbp
       pop      rsi
       pop      rdi
       ret      
************** Beginning of cold code **************
G_M42325_IG12:
       mov      ecx, 3
       call     ThrowHelper:ThrowArgumentNullException(int)
       int3     
; Total bytes of code 193, prolog size 8 for method Array:Sort(ref,int,int,ref)

Post

; Assembly listing for method Array:Sort(ref,int,int,ref)
; Emitting BLENDED_CODE for X64 CPU with SSE2
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 arg0         [V00,T02] (  6,   5  )     ref  ->  rbx        
;  V01 arg1         [V01,T01] (  7,   5.5)     int  ->  rdi        
;  V02 arg2         [V02,T00] (  7,   6  )     int  ->  rsi        
;  V03 arg3         [V03,T03] (  5,   3.5)     ref  ->  rbp        
;  V04 tmp0         [V04,T04] (  2,   2  )     ref  ->  rcx        
;  V05 OutArgs      [V05    ] (  1,   1  )  lclBlk (40) [rsp+0x00]  
;
; Lcl frame size = 40
G_M491_IG01:
       push     rdi
       push     rsi
       push     rbp
       push     rbx
       sub      rsp, 40
       mov      rbx, rcx
       mov      edi, edx
       mov      esi, r8d
       mov      rbp, r9
G_M491_IG02:
       test     rbx, rbx
       je       G_M491_IG12
G_M491_IG03:
       test     edi, edi
       jl       G_M491_IG13
G_M491_IG04:
       test     esi, esi
       jl       G_M491_IG14
G_M491_IG05:
       mov      ecx, dword ptr [rbx+8]
       sub      ecx, edi
       cmp      ecx, esi
       jl       G_M491_IG15
G_M491_IG06:
       cmp      esi, 1
       jle      SHORT G_M491_IG10
       test     rbp, rbp
       je       SHORT G_M491_IG07
       mov      rcx, qword ptr [(reloc)]
       mov      edx, 148
       call     CORINFO_HELP_GETSHARED_GCSTATIC_BASE_DYNAMICCLASS
       cmp      gword ptr [rax], rbp
       jne      SHORT G_M491_IG09
G_M491_IG07:
       lea      r9d, [rdi+rsi-1]
       mov      rcx, rbx
       mov      r8d, edi
       xor      rdx, rdx
       call     Array:TrySZSort(ref,ref,int,int):bool
       test     eax, eax
       je       SHORT G_M491_IG09
G_M491_IG08:
       add      rsp, 40
       pop      rbx
       pop      rbp
       pop      rsi
       pop      rdi
       ret      
G_M491_IG09:
       call     ArraySortHelper`1:get_Default():ref
       mov      rcx, rax
       mov      gword ptr [rsp+20H], rbp
       mov      rdx, rbx
       mov      r8d, edi
       mov      r9d, esi
       lea      r11, [(reloc)]
       cmp      dword ptr [rcx], ecx
       call     qword ptr [r11]IArraySortHelper`1:Sort(ref,int,int,ref):this
G_M491_IG10:
       nop      
G_M491_IG11:
       add      rsp, 40
       pop      rbx
       pop      rbp
       pop      rsi
       pop      rdi
       ret      
************** Beginning of cold code **************
G_M491_IG12:
       mov      ecx, 3
       call     ThrowHelper:ThrowArgumentNullException(int)
G_M491_IG13:
       call     ThrowHelper:ThrowIndexArgumentOutOfRange_NeedNonNegNumException()
G_M491_IG14:
       call     ThrowHelper:ThrowLengthArgumentOutOfRange_ArgumentOutOfRange_NeedNonNegNum()
G_M491_IG15:
       mov      ecx, 23
       call     ThrowHelper:ThrowArgumentException(int)
       int3     
; Total bytes of code 195, prolog size 8 for method Array:Sort(ref,int,int,ref)

benaadams · 2016-10-12T08:54:23Z

With everything being moved to cold code repeats are much more recognisable, for example the bottom of EventProvider:WriteEvent(byref,long,long,ref):bool:this looks like this:

       ret      
************** Beginning of cold code **************
G_M55934_IG93:
       call     CORINFO_HELP_OVERFLOW
G_M55934_IG94:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG95:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG96:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG97:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG98:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG99:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG100:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG101:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG102:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG103:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG104:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG105:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG106:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG107:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG108:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG109:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG110:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG111:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG112:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG113:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG114:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG115:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG116:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG117:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG118:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG119:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG120:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG121:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG122:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG123:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG124:
       call     CORINFO_HELP_RNGCHKFAIL
       int3     
; Total bytes of code 3681, prolog size 61 for method EventProvider:WriteEvent(byref,long,long,ref):bool:this

Not sure what issue to raise for that?

benaadams · 2016-10-12T08:56:13Z

@dotnet-bot test Linux ARM Emulator Cross Debug Build

benaadams · 2016-10-12T09:01:18Z

As before List1:CopyTo(int,ref,int,int):this` is a good example of the effect of do not return now being recognised

Before

; Assembly listing for method List`1:CopyTo(int,ref,int,int):this
; Emitting BLENDED_CODE for X64 CPU with SSE2
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 this         [V00,T00] (  4,   4  )     ref  ->  rsi         this
;  V01 arg1         [V01,T01] (  4,   4  )     int  ->  rdi        
;  V02 arg2         [V02,T02] (  3,   3  )     ref  ->  rbx        
;  V03 arg3         [V03,T03] (  3,   3  )     int  ->  rbp        
;  V04 arg4         [V04,T05] (  2,   2  )     int  ->  r14        
;  V05 tmp0         [V05,T04] (  2,   4  )     ref  ->  rcx        
;  V06 OutArgs      [V06    ] (  1,   1  )  lclBlk (48) [rsp+0x00]  
;
; Lcl frame size = 48
G_M18736_IG01:
       push     r14
       push     rdi
       push     rsi
       push     rbp
       push     rbx
       sub      rsp, 48
       mov      rsi, rcx
       mov      edi, edx
       mov      rbx, r8
       mov      ebp, r9d
       mov      r14d, dword ptr [rsp+80H]
G_M18736_IG02:
       mov      ecx, dword ptr [rsi+24]
       sub      ecx, edi
       cmp      ecx, r14d
       jge      SHORT G_M18736_IG03
       mov      ecx, 23
       call     ThrowHelper:ThrowArgumentException(int)
G_M18736_IG03:
       mov      rcx, gword ptr [rsi+8]
       mov      dword ptr [rsp+20H], r14d
       xor      edx, edx
       mov      dword ptr [rsp+28H], edx
       mov      edx, edi
       mov      r8, rbx
       mov      r9d, ebp
       call     Array:Copy(ref,int,ref,int,int,bool)
       nop      
G_M18736_IG04:
       add      rsp, 48
       pop      rbx
       pop      rbp
       pop      rsi
       pop      rdi
       pop      r14
       ret      
; Total bytes of code 89, prolog size 10 for method List`1:CopyTo(int,ref,int,int):this
; ============================================================

After

; Assembly listing for method List`1:CopyTo(int,ref,int,int):this
; Emitting BLENDED_CODE for X64 CPU with SSE2
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 this         [V00,T00] (  4,   4  )     ref  ->  rcx         this
;  V01 arg1         [V01,T01] (  4,   4  )     int  ->  rdx        
;  V02 arg2         [V02,T02] (  3,   3  )     ref  ->   r8        
;  V03 arg3         [V03,T03] (  3,   3  )     int  ->   r9        
;  V04 arg4         [V04,T05] (  2,   2  )     int  ->  rax        
;  V05 tmp0         [V05,T04] (  2,   4  )     ref  ->  rcx        
;  V06 OutArgs      [V06    ] (  1,   1  )  lclBlk (48) [rsp+0x00]  
;
; Lcl frame size = 56
G_M45634_IG01:
       sub      rsp, 56
       nop      
       mov      eax, dword ptr [rsp+60H]
G_M45634_IG02:
       mov      r10d, dword ptr [rcx+24]
       sub      r10d, edx
       cmp      r10d, eax
       jl       G_M45634_IG05
G_M45634_IG03:
       mov      rcx, gword ptr [rcx+8]
       mov      dword ptr [rsp+20H], eax
       xor      eax, eax
       mov      dword ptr [rsp+28H], eax
       call     Array:Copy(ref,int,ref,int,int,bool)
       nop      
G_M45634_IG04:
       add      rsp, 56
       ret      
************** Beginning of cold code **************
G_M45634_IG05:
       mov      ecx, 23
       call     ThrowHelper:ThrowArgumentException(int)
       int3     
; Total bytes of code 61, prolog size 5 for method List`1:CopyTo(int,ref,int,int):this
; ============================================================

benaadams · 2016-10-12T09:22:15Z

ArraySegment1:.ctor(ref,int,int):this` is an example of the slight expansion from recognising cold code; which is then multiplied due to the methods being generic. (Though I would argue the code quality has improved)

Before

; Assembly listing for method ArraySegment`1:.ctor(ref,int,int):this
; Emitting BLENDED_CODE for X64 CPU with SSE2
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 this         [V00,T00] (  5,   5  )   byref  ->  rsi         this
;* V01 TypeCtx      [V01    ] (  0,   0  )    long  ->  zero-ref   
;  V02 arg1         [V02,T01] (  5,   5  )     ref  ->  rdi        
;  V03 arg2         [V03,T02] (  5,   5  )     int  ->  rbx        
;  V04 arg3         [V04,T03] (  3,   3  )     int  ->  rbp        
;  V05 loc0         [V05    ] (  1,   1  )  lclBlk (32) [rsp+0x00]  
;
; Lcl frame size = 40
G_M41507_IG01:
       push     rdi
       push     rsi
       push     rbp
       push     rbx
       sub      rsp, 40
       mov      rsi, rcx
       mov      rdi, r8
       mov      ebx, r9d
       mov      ebp, dword ptr [rsp+70H]
G_M41507_IG02:
       test     rdi, rdi
       je       G_M41507_IG08
G_M41507_IG03:
       test     ebx, ebx
       jge      SHORT G_M41507_IG04
       mov      ecx, 26
       mov      edx, 4
       call     ThrowHelper:ThrowArgumentOutOfRangeException(int,int)
G_M41507_IG04:
       test     ebp, ebp
       jge      SHORT G_M41507_IG05
       mov      ecx, 16
       mov      edx, 4
       call     ThrowHelper:ThrowArgumentOutOfRangeException(int,int)
G_M41507_IG05:
       mov      ecx, dword ptr [rdi+8]
       sub      ecx, ebx
       cmp      ecx, ebp
       jge      SHORT G_M41507_IG06
       mov      ecx, 23
       call     ThrowHelper:ThrowArgumentException(int)
G_M41507_IG06:
       mov      rcx, rsi
       mov      rdx, rdi
       call     CORINFO_HELP_CHECKED_ASSIGN_REF
       mov      dword ptr [rsi+8], ebx
       mov      dword ptr [rsi+12], ebp
G_M41507_IG07:
       add      rsp, 40
       pop      rbx
       pop      rbp
       pop      rsi
       pop      rdi
       ret      
************** Beginning of cold code **************
G_M41507_IG08:
       mov      ecx, 3
       call     ThrowHelper:ThrowArgumentNullException(int)
       int3     
; Total bytes of code 124, prolog size 8 for method ArraySegment`1:.ctor(ref,int,int):this
; ============================================================

After

; Assembly listing for method ArraySegment`1:.ctor(ref,int,int):this
; Emitting BLENDED_CODE for X64 CPU with SSE2
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 this         [V00,T00] (  5,   5  )   byref  ->  rsi         this
;* V01 TypeCtx      [V01    ] (  0,   0  )    long  ->  zero-ref   
;  V02 arg1         [V02,T01] (  5,   5  )     ref  ->   r8        
;  V03 arg2         [V03,T02] (  5,   5  )     int  ->  rdi        
;  V04 arg3         [V04,T03] (  3,   3  )     int  ->  rbx        
;  V05 loc0         [V05    ] (  1,   1  )  lclBlk (32) [rsp+0x00]  
;
; Lcl frame size = 32
G_M41510_IG01:
       push     rdi
       push     rsi
       push     rbx
       sub      rsp, 32
       mov      rsi, rcx
       mov      edi, r9d
       mov      ebx, dword ptr [rsp+60H]
G_M41510_IG02:
       test     r8, r8
       je       G_M41510_IG08
G_M41510_IG03:
       test     edi, edi
       jl       G_M41510_IG09
G_M41510_IG04:
       test     ebx, ebx
       jl       G_M41510_IG10
G_M41510_IG05:
       mov      edx, dword ptr [r8+8]
       sub      edx, edi
       cmp      edx, ebx
       jl       G_M41510_IG11
G_M41510_IG06:
       mov      rcx, rsi
       mov      rdx, r8
       call     CORINFO_HELP_CHECKED_ASSIGN_REF
       mov      dword ptr [rsi+8], edi
       mov      dword ptr [rsi+12], ebx
G_M41510_IG07:
       add      rsp, 32
       pop      rbx
       pop      rsi
       pop      rdi
       ret      
************** Beginning of cold code **************
G_M41510_IG08:
       mov      ecx, 3
       call     ThrowHelper:ThrowArgumentNullException(int)
G_M41510_IG09:
       mov      ecx, 26
       mov      edx, 4
       call     ThrowHelper:ThrowArgumentOutOfRangeException(int,int)
G_M41510_IG10:
       mov      ecx, 16
       mov      edx, 4
       call     ThrowHelper:ThrowArgumentOutOfRangeException(int,int)
G_M41510_IG11:
       mov      ecx, 23
       call     ThrowHelper:ThrowArgumentException(int)
       int3     
; Total bytes of code 132, prolog size 7 for method ArraySegment`1:.ctor(ref,int,int):this
; ============================================================

jkotas · 2016-10-12T12:56:34Z

@dotnet-bot test this please

AndyAyersMS · 2016-10-12T20:49:21Z

G_M55934_IG94:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
G_M55934_IG95:
       call     ThrowHelper:ThrowArgumentOutOfRange_IndexException()
...

If the jit is generating native offset to IL tracking data then the repeated code sequences like the above will typically have different IL offsets and so can't be merged without losing track of which check caused an exception.

benaadams · 2016-10-12T20:58:14Z

so can't be merged without losing track of which check caused an exception.

Ah, makes sense.

benaadams · 2016-10-12T21:05:53Z

src/mscorlib/src/System/Collections/ObjectModel/ReadOnlyCollection.cs

@@ -128,7 +128,7 @@ public class ReadOnlyCollection<T>: IList<T>, IList, IReadOnlyList<T>
            }

            if (index < 0) {
-                ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.arrayIndex, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum);
+                ThrowHelper.ThrowIndexArgumentOutOfRange_NeedNonNegNumException();


Was previously arrayIndex rather than index but that doesn't actually match the parameter

AndyAyersMS · 2016-10-12T22:31:48Z

src/jit/flowgraph.cpp

@@ -5763,7 +5772,7 @@ void Compiler::fgFindBasicBlocks()
            impInlineInfo->iciCall->gtCallMoreFlags |= GTF_CALL_M_DOES_NOT_RETURN;


This flag setting is redundant now, isn't it? The assert above could now check either there are no return blocks and the flag was set, OR, there are return blocks and the flag is not set.

Mean the one in fgFindBasicBlocks? Could be less returns in there than fgFindJumpTargets sees as unreachable returns could be removed between the two?

The issue I'm trying to resolve is Compiler::compCompileHelper (I think its this one) runs much earlier and does pre-screening during crossgen and marks some candidates as non-inlinable early so fgMakeBasicBlocks never gets run with inlining and the GTF_CALL_M_DOES_NOT_RETURN flag therefore never gets set for these methods.

Although I think shrinking the ThrowHelper methods may have also resolved this

Right, earlier detection is a good idea, since we'd rather fail with the no return reason than some other reason. I am just asking if it means the later detection becomes redundant (I suspect it does since not much happens in between).

Ah yeah its just adjusts jump targets for exception handlers, doesn't actually change any of the returns. Probably should move the observation up also then leave the assert.

Yep, results are exactly the same removing the second check

Summary: (Note: Lower is better) Total bytes of diff: -11727 (-0.37 % of base) diff is an improvement. Total byte diff includes 1328 bytes from reconciling methods Base had 1 unique methods, 41 unique bytes Diff had 19 unique methods, 1369 unique bytes Top file improvements by size (bytes): -11727 : System.Private.CoreLib.dasm (-0.37 % of base) 1 total files with size differences. Top method regessions by size (bytes): 181 : System.Private.CoreLib.dasm - ThrowHelper:GetArgumentOutOfRangeException(int,int,int):ref 173 : System.Private.CoreLib.dasm - Dictionary`2:OnDeserialization(ref):this (29 methods) 126 : System.Private.CoreLib.dasm - Comparer`1:System.Collections.IComparer.Compare(ref,ref):int:this (26 methods) 125 : System.Private.CoreLib.dasm - ThrowHelper:GetWrongKeyTypeArgumentException(ref,ref):ref 125 : System.Private.CoreLib.dasm - ThrowHelper:GetWrongValueTypeArgumentException(ref,ref):ref Top method improvements by size (bytes): -1588 : System.Private.CoreLib.dasm - Enumerator:MoveNext():bool:this (113 methods) -945 : System.Private.CoreLib.dasm - EventProvider:WriteEvent(byref,long,long,ref):bool:this -904 : System.Private.CoreLib.dasm - Enumerator:System.Collections.IEnumerator.Reset():this (113 methods) -644 : System.Private.CoreLib.dasm - List`1:CopyTo(int,ref,int,int):this (23 methods) -598 : System.Private.CoreLib.dasm - List`1:FindLastIndex(int,int,ref):int:this (23 methods) 660 total methods with size differences.

benaadams · 2016-10-13T04:41:20Z

src/jit/flowgraph.cpp

@@ -5735,35 +5760,10 @@ void Compiler::fgFindBasicBlocks()
            return;
        }


Don't need both failure checks

benaadams · 2016-10-13T06:36:47Z

@AndyAyersMS resolved?

AndyAyersMS · 2016-10-13T15:56:04Z

Yes, LGTM. Thanks!

…turn-early Flag does_not_return early Commit migrated from dotnet/coreclr@ff3ab00

dnfclas added the cla-already-signed label Oct 12, 2016

benaadams mentioned this pull request Oct 12, 2016

Set DOES_NOT_RETURN gtCallMoreFlags in fgFindJumpTargets #6742

Closed

benaadams commented Oct 12, 2016

View reviewed changes

AndyAyersMS reviewed Oct 12, 2016

View reviewed changes

benaadams force-pushed the flag-doesnot-return-early branch from 2c6d733 to 145da53 Compare October 13, 2016 00:54

benaadams commented Oct 13, 2016

View reviewed changes

benaadams added 8 commits October 13, 2016 05:51

Count BBJ_RETURN in fgMakeBasicBlocks

fef8b0b

Set DOES_NOT_RETURN flag in fgFindJumpTargets

d5796e9

Make ThrowHelper paths simple for DOES_NOT_RETURN

8e7d790

Use existing common exceptions

f83e718

Common Enumerator exceptions

b9fb8dc

Common CopyTo Exceptions

5504783

Common List+Array Exceptions

2965a50

Move up DOES_NOT_RETURN more

29d7af4

benaadams force-pushed the flag-doesnot-return-early branch from 4bce457 to 29d7af4 Compare October 13, 2016 04:52

AndyAyersMS merged commit ff3ab00 into dotnet:master Oct 14, 2016

benaadams deleted the flag-doesnot-return-early branch October 14, 2016 18:21

karelz modified the milestone: 2.0.0 Aug 28, 2017

This was referenced Nov 27, 2017

Key item added to KeyNotFoundException #15201

Merged

Address PR feedback for change adding key to KeyNotFoundException #15234

Merged

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Merge pull request dotnet/coreclr#7580 from benaadams/flag-doesnot-re…

6fbf65d

…turn-early Flag does_not_return early Commit migrated from dotnet/coreclr@ff3ab00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flag does_not_return early #7580

Flag does_not_return early #7580

benaadams commented Oct 12, 2016 •

edited

Loading

benaadams commented Oct 12, 2016

benaadams commented Oct 12, 2016 •

edited

Loading

benaadams commented Oct 12, 2016

benaadams commented Oct 12, 2016

benaadams commented Oct 12, 2016 •

edited

Loading

benaadams commented Oct 12, 2016 •

edited

Loading

jkotas commented Oct 12, 2016

AndyAyersMS commented Oct 12, 2016

benaadams commented Oct 12, 2016 •

edited

Loading

benaadams Oct 12, 2016

AndyAyersMS Oct 12, 2016

benaadams Oct 12, 2016

benaadams Oct 12, 2016

AndyAyersMS Oct 12, 2016

benaadams Oct 12, 2016

benaadams Oct 13, 2016 •

edited

Loading

benaadams Oct 13, 2016

benaadams Oct 13, 2016

benaadams commented Oct 13, 2016

AndyAyersMS commented Oct 13, 2016

		@@ -5763,7 +5772,7 @@ void Compiler::fgFindBasicBlocks()
		impInlineInfo->iciCall->gtCallMoreFlags \|= GTF_CALL_M_DOES_NOT_RETURN;

		@@ -5735,35 +5760,10 @@ void Compiler::fgFindBasicBlocks()
		return;
		}

Flag does_not_return early #7580

Flag does_not_return early #7580

Conversation

benaadams commented Oct 12, 2016 • edited Loading

benaadams commented Oct 12, 2016

benaadams commented Oct 12, 2016 • edited Loading

benaadams commented Oct 12, 2016

benaadams commented Oct 12, 2016

benaadams commented Oct 12, 2016 • edited Loading

benaadams commented Oct 12, 2016 • edited Loading

jkotas commented Oct 12, 2016

AndyAyersMS commented Oct 12, 2016

benaadams commented Oct 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benaadams Oct 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benaadams commented Oct 13, 2016

AndyAyersMS commented Oct 13, 2016

benaadams commented Oct 12, 2016 •

edited

Loading

benaadams commented Oct 12, 2016 •

edited

Loading

benaadams commented Oct 12, 2016 •

edited

Loading

benaadams commented Oct 12, 2016 •

edited

Loading

benaadams commented Oct 12, 2016 •

edited

Loading

benaadams Oct 13, 2016 •

edited

Loading