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

Conversation

@JosephTremoulet
Copy link

Remove some gotos that were added to work around undesirable jit
layout (#9692, fixed in #13314) and epilog factoring (improved in
#13792 and #13903), which are no longer needed.

Resolves #13466.

Remove some `goto`s that were added  to work around undesirable jit
layout (#9692, fixed in dotnet#13314) and epilog factoring (improved in
 dotnet#13792 and dotnet#13903), which are no longer needed.

Resolves #13466.
@JosephTremoulet
Copy link
Author

@stephentoub / @jkotas PTAL.

The machine code generated by crossgen of SPCLib is identical before/after this change for EqualsHelper. For StartsWithOrdinalHelper, there is an extra movzx due to some known issues with the JIT's modeling of smaller-than-32-bit values:

diff --git "a/.\\StartsWithOrdinalHelper-before.dasm" "b/.\\StartsWithOrdinalHelper-after.dasm"
index c8c3551..b38eb8b 100644
--- "a/.\\StartsWithOrdinalHelper-before.dasm"
+++ "b/.\\StartsWithOrdinalHelper-after.dasm"
@@ -12,9 +12,10 @@
 ;  V04 loc2         [V04    ] (  2,  2   )   byref  ->  [rsp+0x08]   must-init pinned
 ;  V05 loc3         [V05,T00] ( 13, 35.50)    long  ->  rdx        
 ;  V06 loc4         [V06,T01] ( 13, 35.50)    long  ->  rcx        
-;  V07 tmp0         [V07,T05] (  2,  4   )    long  ->  rdx        
-;  V08 tmp1         [V08,T06] (  2,  4   )    long  ->  rcx        
-;# V09 OutArgs      [V09    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]  
+;  V07 tmp0         [V07,T05] (  2,  2   )     int  ->  rax        
+;  V08 tmp1         [V08,T06] (  2,  4   )    long  ->  rdx        
+;  V09 tmp2         [V09,T07] (  2,  4   )    long  ->  rcx        
+;# V10 OutArgs      [V10    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]  
 ;
 ; Lcl frame size = 24
 
@@ -35,37 +36,47 @@ G_M6815_IG02:
        488B4C2408           mov      rcx, bword ptr [rsp+08H]
        448B02               mov      r8d, dword ptr [rdx]
        443B01               cmp      r8d, dword ptr [rcx]
-       7574                 jne      SHORT G_M6815_IG08
+       7577                 jne      SHORT G_M6815_IG14
+
+G_M6815_IG02B:
        83C0FE               add      eax, -2
        4883C204             add      rdx, 4
        4883C104             add      rcx, 4
        83F80C               cmp      eax, 12
-       7C2C                 jl       SHORT G_M6815_IG04
+       7C2C                 jl       SHORT G_M6815_IG08
 
-G_M6815_IG03:
+G_M6815_IG04:
        4C8B02               mov      r8, qword ptr [rdx]
        4C3B01               cmp      r8, qword ptr [rcx]
-       755C                 jne      SHORT G_M6815_IG08
+       755F                 jne      SHORT G_M6815_IG14
+
+G_M6815_IG03B:
        4C8B4208             mov      r8, qword ptr [rdx+8]
        4C3B4108             cmp      r8, qword ptr [rcx+8]
-       7552                 jne      SHORT G_M6815_IG08
+       7555                 jne      SHORT G_M6815_IG14
+
+G_M6815_IG03C:
        4C8B4210             mov      r8, qword ptr [rdx+16]
        4C3B4110             cmp      r8, qword ptr [rcx+16]
-       7548                 jne      SHORT G_M6815_IG08
+       754B                 jne      SHORT G_M6815_IG14
+
+G_M6815_IG03D:
        83C0F4               add      eax, -12
        4883C218             add      rdx, 24
        4883C118             add      rcx, 24
        83F80C               cmp      eax, 12
-       7DD4                 jge      SHORT G_M6815_IG03
+       7DD4                 jge      SHORT G_M6815_IG04
 
-G_M6815_IG04:
+G_M6815_IG08:
        83F802               cmp      eax, 2
-       7C18                 jl       SHORT G_M6815_IG06
+       7C18                 jl       SHORT G_M6815_IG11
 
 G_M6815_IG05:
        448B02               mov      r8d, dword ptr [rdx]
        443B01               cmp      r8d, dword ptr [rcx]
-       752B                 jne      SHORT G_M6815_IG08
+       752E                 jne      SHORT G_M6815_IG08
+
+G_M6815_IG05B:
        83C0FE               add      eax, -2
        4883C204             add      rdx, 4
        4883C104             add      rcx, 4
@@ -83,6 +94,9 @@ G_M6815_IG06:
        0BC2                 or       eax, edx
 
 G_M6815_IG07:
+       0FB6C0               movzx    rax, al   ; <-- this is new
+
+G_M6815_IG07B:
        4883C418             add      rsp, 24
        C3                   ret      
 
@@ -93,7 +107,7 @@ G_M6815_IG09:
        4883C418             add      rsp, 24
        C3                   ret      
 
-; Total bytes of code 180, prolog size 16 for method String:StartsWithOrdinalHelper(ref,ref):bool
+; Total bytes of code 183, prolog size 16 for method String:StartsWithOrdinalHelper(ref,ref):bool

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @JosephTremoulet.

@JosephTremoulet
Copy link
Author

@dotnet-bot test Windows_NT arm Cross Checked Build and Test (a known issue was just resolved)

@jkotas jkotas merged commit 1376d3b into dotnet:master Sep 14, 2017
@JosephTremoulet JosephTremoulet deleted the GotoUndo branch September 14, 2017 13:57
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.

Undo "goto return" works-around

4 participants