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

JIT: Bounds checks not removed after casting a Span<T> with a known length to another type #32776

Closed
Thealexbarney opened this issue Feb 25, 2020 · 2 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI JitUntriaged CLR JIT issues needing additional triage
Milestone

Comments

@Thealexbarney
Copy link

Thealexbarney commented Feb 25, 2020

The JIT doesn't completely eliminate the bounds checks in either of these functions.

public static short GetValue()
{
    ReadOnlySpan<byte> span = new byte[] { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
    return MemoryMarshal.Cast<byte, short>(span)[1];
}

public static short GetValueS(ref byte a)
{
    ReadOnlySpan<byte> span = MemoryMarshal.CreateReadOnlySpan(ref a, 6);
    return MemoryMarshal.Cast<byte, short>(span)[1];
}

It's able to recognize that the new span has a known length, but doesn't remove the comparison between constants.

G_M41738_IG01:
       sub      rsp, 40
       nop      

G_M41738_IG02:
       mov      eax, 3
       cmp      eax, 1
       jbe      SHORT G_M41738_IG05
       mov      rax, 0xD1FFAB1E
       movsx    rax, word  ptr [rax]

G_M41738_IG03:
       add      rsp, 40
       ret      

G_M41738_IG04:
       call     CORINFO_HELP_OVERFLOW
       int3     

G_M41738_IG05:
       call     CORINFO_HELP_RNGCHKFAIL
       int3     

; Total bytes of code 46

-------------------------------------
G_M41737_IG01:
       sub      rsp, 40
       nop      

G_M41737_IG02:
       mov      eax, 3
       cmp      eax, 1
       jbe      SHORT G_M41737_IG05
       movsx    rax, word  ptr [rcx+2]

G_M41737_IG03:
       add      rsp, 40
       ret      

G_M41737_IG04:
       call     CORINFO_HELP_OVERFLOW
       int3     

G_M41737_IG05:
       call     CORINFO_HELP_RNGCHKFAIL
       int3     

; Total bytes of code 37

Returning a byte instead of casting to a short results in no bounds checks.

G_M41729_IG01:
       nop      

G_M41729_IG02:
       mov      rax, 0xD1FFAB1E
       movzx    rax, byte  ptr [rax]

G_M41729_IG03:
       ret      

; Total bytes of code 19

-------------------------------------
G_M41728_IG01:
       nop      

G_M41728_IG02:
       movzx    rax, byte  ptr [rcx+1]

G_M41728_IG03:
       ret      

; Total bytes of code 10

category:cq
theme:bounds-checks
skill-level:expert
cost:medium

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Feb 25, 2020
@gfoidl
Copy link
Member

gfoidl commented Feb 25, 2020

Aside from the issue, a note of caution:

return MemoryMarshal.Cast<byte, short>(span)[1];

is not endianess-safe, i.e. as little-endian user on big-endian machines you won't get the expected short value.
(That's also the reason why the ROS-static data trick by the C# compiler only works with byte, bool).

@jakobbotsch
Copy link
Member

Codegen on main:

; Method Program:GetValue():short
G_M6304_IG01:
       sub      rsp, 40
						;; size=4 bbWeight=1    PerfScore 0.25

G_M6304_IG02:
       mov      rax, 0xD1FFAB1E      ; data for <PrivateImplementationDetails>:B0F66ADC83641586656866813FD9DD0B8EBB63796075661BA45D1AA8089E1D44
       movsx    rax, word  ptr [rax+02H]
						;; size=15 bbWeight=1    PerfScore 4.25

G_M6304_IG03:
       add      rsp, 40
       ret      
						;; size=5 bbWeight=1    PerfScore 1.25

G_M6304_IG04:
       call     CORINFO_HELP_OVERFLOW
       int3     
						;; size=6 bbWeight=0    PerfScore 0.00
; Total bytes of code: 30
; Method Program:GetValueS(byref):short
G_M36409_IG01:
       sub      rsp, 40
						;; size=4 bbWeight=1    PerfScore 0.25

G_M36409_IG02:
       movsx    rax, word  ptr [rcx+02H]
						;; size=5 bbWeight=1    PerfScore 4.00

G_M36409_IG03:
       add      rsp, 40
       ret      
						;; size=5 bbWeight=1    PerfScore 1.25

G_M36409_IG04:
       call     CORINFO_HELP_OVERFLOW
       int3     
						;; size=6 bbWeight=0    PerfScore 0.00
; Total bytes of code: 20

Looks fixed.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI JitUntriaged CLR JIT issues needing additional triage
Projects
None yet
Development

No branches or pull requests

5 participants