Remove redundant ulong casts in Span.Slice for x64 targets

[reedz]

Fixes #120607.

Related: #67044 , #119689

Remove 64-bit workarounds (ulong casts for 64-bit arithmetic) to produce JIT-friendly code that eliminates unnecessary bounds checks for Span slices. Presumably, this has become possible thanks to earlier work done in #62864,

Code from the issue:

[MethodImpl(MethodImplOptions.NoInlining)]
static Span<byte> SlicePattern(Span<byte> input)
{
    if (input.Length >= 1)
        return input.Slice(1, input.Length - 1);  // Should have no bounds check
    return input;
}

Before changes:

G_M000_IG01:
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp

G_M000_IG02:
            cmp     w1, #0
            bgt     G_M000_IG04

G_M000_IG03:
            ldp     fp, lr, [sp], #0x10
            ret     lr

G_M000_IG04:
            sub     w2, w1, #1          ; compute length - 1
            mov     w3, w2
            add     x3, x3, #1          ; compute 1 + (length - 1)
            cmp     x3, w1, UXTW        ; BOUNDS CHECK: compare with original length
            bhi     G_M000_IG06         ; branch to throw if out of bounds
            add     x0, x0, #1
            mov     x1, x2

G_M000_IG05:
            ldp     fp, lr, [sp], #0x10
            ret     lr

G_M000_IG06:                            ; throw path
            movz    x0, #0xD908
            movk    x0, #0x1012 LSL #16
            movk    x0, #0xEDC5 LSL #32
            ldr     x0, [x0]
            blr     x0
            brk     #0

; Total bytes of code 84

After changes

G_M000_IG01:
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp

G_M000_IG02:
            cmp     w1, #0              ; if (input.Length >= 1)
            bgt     G_M000_IG04

G_M000_IG03:
            ldp     fp, lr, [sp], #0x10
            ret     lr

G_M000_IG04:
            sub     w1, w1, #1          ; new length = length - 1
            add     x0, x0, #1          ; new pointer = pointer + 1

G_M000_IG05:
            ldp     fp, lr, [sp], #0x10
            ret     lr

; Total bytes of code 40

[Copilot]

Pull request overview

This PR removes platform-specific bounds checking workarounds for 64-bit targets in Span, Memory, and related types. The change simplifies the code by using a uniform bounds check pattern across all platforms, enabling better JIT optimization and eliminating unnecessary bounds checks in common slicing patterns.

Changes:

• Replaced complex 64-bit-specific bounds checks with simpler, JIT-friendly two-condition checks that work uniformly across all platforms
• Updated comment in ValueListBuilder.cs to reflect that the guard condition is now universal rather than 64-bit specific
• Removed all TARGET_64BIT conditional compilation directives related to bounds checking from Span, Memory, String, and related types

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/Span.cs	Removed TARGET_64BIT conditional for Slice bounds checking in both constructor and Slice method
src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs	Removed TARGET_64BIT conditional for Slice bounds checking in both constructor and Slice method
src/libraries/System.Private.CoreLib/src/System/Memory.cs	Removed TARGET_64BIT conditional for Slice bounds checking in constructor, Slice method, and Span property getter
src/libraries/System.Private.CoreLib/src/System/ReadOnlyMemory.cs	Removed TARGET_64BIT conditional for Slice bounds checking in constructor, Slice method, and Span property getter
src/libraries/System.Private.CoreLib/src/System/String.cs	Removed TARGET_64BIT conditional from TryGetSpan method
src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs	Removed TARGET_64BIT conditional from Substring method
src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs	Removed TARGET_64BIT conditionals from AsSpan and AsMemory extension methods
src/libraries/System.Private.CoreLib/src/System/Collections/Generic/ValueListBuilder.cs	Updated comment to reflect universal application of guard condition and simplified the bounds check

[EgorBo]

you need the !TARGET_64BIT path from Slice here, this is not the same as before

[reedz]

Thanks, this has been reverted.
@EgorBo is it possible we're seeing multiple regressions because of this change ? Would you mind re-running MihuBot once more for the fresh changes ? It seems I don't have access, and I'm not sure how I can run the same suite locally.

[EgorBo]

Some of my recent JIT changes were preparing for this actually, but I am not yet sure it won't regress today.

#if TARGET_64BIT
            // See comment in Span<T>.Slice for how this works.
            if ((ulong)(uint)start + (ulong)(uint)length > (ulong)(uint)_length)
                ThrowHelper.ThrowArgumentOutOfRangeException();
#else
            if ((uint)start > (uint)_length || (uint)length > (uint)(_length - start))
                ThrowHelper.ThrowArgumentOutOfRangeException();
#endif

the 64bit path messes with JIT's range check analysis which is built mostly around 32-bit integers (because of Array/Span Length being so), but at the same time it's just one condition+branch. The latter produces two jumps and JIT is not smart enough to merge it into one when it can (after opts).

[EgorBo]

@MihuBot

[EgorBo]

1512 improved, 2417 regressed

I think it's not ready for merge yet, needs more work in JIT