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

Fewer instructions for check if either start or length != 0 #23239

Closed
wants to merge 1 commit into from
Closed

Fewer instructions for check if either start or length != 0 #23239

wants to merge 1 commit into from

Conversation

gfoidl
Copy link
Member

@gfoidl gfoidl commented Mar 13, 2019

@gfoidl gfoidl mentioned this pull request Mar 13, 2019
@stephentoub
Copy link
Member

@gfoidl
Copy link
Member Author

gfoidl commented Mar 13, 2019

Ah, the JIT is smarter than expected, as it does this optimization for us.
Now I'm just interested how it plays together with https://github.com/dotnet/coreclr/issues/19074.

For verifying I used code from

public Memory(T[] array, int start, int length)

Test-Code
using System;
using System.Runtime.CompilerServices;

namespace ConsoleApp3
{
    class Program
    {
        static void Main(string[] args)
        {
            var array = new byte[] { 42 };
            int start = 0;
            int length = 1;

            Memory0<byte> m0 = Default(array, start, length);
            Memory1<byte> m1 = PR(array, start, length);
        }

        [MethodImpl(MethodImplOptions.NoInlining)]
        private static Memory0<byte> Default(byte[] array, int start, int length)
        {
            return new Memory0<byte>(array, start, length);
        }

        [MethodImpl(MethodImplOptions.NoInlining)]
        private static Memory1<byte> PR(byte[] array, int start, int length)
        {
            return new Memory1<byte>(array, start, length);
        }
    }

    public readonly struct Memory0<T>
    {
        private readonly object _object;
        private readonly int _index;
        private readonly int _length;

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public Memory0(T[] array, int start, int length)
        {
            if (array == null)
            {
                if (start != 0 || length != 0)
                    ThrowHelper.ThrowArgumentOutOfRangeException();

                this = default;
                return;
            }

            if (default(T) == null && array.GetType() != typeof(T[]))
                ThrowHelper.ThrowArrayTypeMismatchException();

            if ((ulong)(uint)start + (ulong)(uint)length > (ulong)(uint)array.Length)
                ThrowHelper.ThrowArgumentOutOfRangeException();

            _object = array;
            _index = start;
            _length = length;
        }
    }

    public readonly struct Memory1<T>
    {
        private readonly object _object;
        private readonly int _index;
        private readonly int _length;

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public Memory1(T[] array, int start, int length)
        {
            if (array == null)
            {
                if ((start | length) != 0)
                    ThrowHelper.ThrowArgumentOutOfRangeException();

                this = default;
                return;
            }

            if (default(T) == null && array.GetType() != typeof(T[]))
                ThrowHelper.ThrowArrayTypeMismatchException();

            if ((ulong)(uint)start + (ulong)(uint)length > (ulong)(uint)array.Length)
                ThrowHelper.ThrowArgumentOutOfRangeException();

            _object = array;
            _index = start;
            _length = length;
        }
    }

    public static class ThrowHelper
    {
        public static void ThrowArgumentOutOfRangeException() => throw new Exception();
        public static void ThrowArrayTypeMismatchException() => throw new Exception();
    }
}
JITed assembly
; Assembly listing for method Program:Default(ref,int,int):struct
; Emitting BLENDED_CODE for X64 CPU with AVX - Unix
; optimized code
; rbp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 arg0         [V00,T01] (  5,  3.75)     ref  ->  rdi         class-hnd
;  V01 arg1         [V01,T02] (  5,  3   )     int  ->  rsi
;  V02 arg2         [V02,T03] (  5,  3   )     int  ->  rdx
;# V03 OutArgs      [V03    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]   "OutgoingArgSpace"
;  V04 tmp1         [V04,T00] (  6,  8   )  struct (16) [rbp-0x10]   do-not-enreg[SFBR] multireg-ret must-init "NewObj constructor temp"
;* V05 tmp2         [V05    ] (  0,  0   )   ubyte  ->  zero-ref    ld-addr-op "Inline ldloca(s) first use temp"
;* V06 tmp3         [V06    ] (  0,  0   )     ref  ->  zero-ref    class-hnd exact "Single-def Box Helper"
;
; Lcl frame size = 16

G_M46330_IG01:
       55                   push     rbp
       4883EC10             sub      rsp, 16
       C5F877               vzeroupper
       488D6C2410           lea      rbp, [rsp+10H]
       33C0                 xor      rax, rax
       488945F0             mov      qword ptr [rbp-10H], rax

G_M46330_IG02:
       488D45F0             lea      rax, bword ptr [rbp-10H]

G_M46330_IG03:
       C5F857C0             vxorps   xmm0, xmm0
       C5FA7F00             vmovdqu  qword ptr [rax], xmm0

G_M46330_IG04:
       4885FF               test     rdi, rdi
       7514                 jne      SHORT G_M46330_IG08
       0BF2                 or       esi, edx
       7537                 jne      SHORT G_M46330_IG12

G_M46330_IG05:
       488D55F0             lea      rdx, bword ptr [rbp-10H]

G_M46330_IG06:
       C5F857C0             vxorps   xmm0, xmm0
       C5FA7F02             vmovdqu  qword ptr [rdx], xmm0

G_M46330_IG07:
       EB19                 jmp      SHORT G_M46330_IG10

G_M46330_IG08:
       8BC6                 mov      eax, esi
       8BCA                 mov      ecx, edx
       4803C1               add      rax, rcx
       8B4F08               mov      ecx, dword ptr [rdi+8]
       483BC1               cmp      rax, rcx
       771E                 ja       SHORT G_M46330_IG13

G_M46330_IG09:
       48897DF0             mov      gword ptr [rbp-10H], rdi
       8975F8               mov      dword ptr [rbp-08H], esi
       8955FC               mov      dword ptr [rbp-04H], edx

G_M46330_IG10:
       488B45F0             mov      rax, gword ptr [rbp-10H]
       488B55F8             mov      rdx, qword ptr [rbp-08H]

G_M46330_IG11:
       488D6500             lea      rsp, [rbp]
       5D                   pop      rbp
       C3                   ret

G_M46330_IG12:
       E80EF4FFFF           call     ThrowHelper:ThrowArgumentOutOfRangeException()
       CC                   int3

G_M46330_IG13:
       E808F4FFFF           call     ThrowHelper:ThrowArgumentOutOfRangeException()
       CC                   int3

; Total bytes of code 105, prolog size 19 for method Program:Default(ref,int,int):struct
; ============================================================
; Assembly listing for method Program:PR(ref,int,int):struct
; Emitting BLENDED_CODE for X64 CPU with AVX - Unix
; optimized code
; rbp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 arg0         [V00,T01] (  5,  3.75)     ref  ->  rdi         class-hnd
;  V01 arg1         [V01,T02] (  5,  3   )     int  ->  rsi
;  V02 arg2         [V02,T03] (  5,  3   )     int  ->  rdx
;# V03 OutArgs      [V03    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]   "OutgoingArgSpace"
;  V04 tmp1         [V04,T00] (  6,  8   )  struct (16) [rbp-0x10]   do-not-enreg[SFBR] multireg-ret must-init "NewObj constructor temp"
;* V05 tmp2         [V05    ] (  0,  0   )   ubyte  ->  zero-ref    ld-addr-op "Inline ldloca(s) first use temp"
;* V06 tmp3         [V06    ] (  0,  0   )     ref  ->  zero-ref    class-hnd exact "Single-def Box Helper"
;
; Lcl frame size = 16

G_M56801_IG01:
       55                   push     rbp
       4883EC10             sub      rsp, 16
       C5F877               vzeroupper
       488D6C2410           lea      rbp, [rsp+10H]
       33C0                 xor      rax, rax
       488945F0             mov      qword ptr [rbp-10H], rax

G_M56801_IG02:
       488D45F0             lea      rax, bword ptr [rbp-10H]

G_M56801_IG03:
       C5F857C0             vxorps   xmm0, xmm0
       C5FA7F00             vmovdqu  qword ptr [rax], xmm0

G_M56801_IG04:
       4885FF               test     rdi, rdi
       7514                 jne      SHORT G_M56801_IG08
       0BF2                 or       esi, edx
       7537                 jne      SHORT G_M56801_IG12

G_M56801_IG05:
       488D55F0             lea      rdx, bword ptr [rbp-10H]

G_M56801_IG06:
       C5F857C0             vxorps   xmm0, xmm0
       C5FA7F02             vmovdqu  qword ptr [rdx], xmm0

G_M56801_IG07:
       EB19                 jmp      SHORT G_M56801_IG10

G_M56801_IG08:
       8BC6                 mov      eax, esi
       8BCA                 mov      ecx, edx
       4803C1               add      rax, rcx
       8B4F08               mov      ecx, dword ptr [rdi+8]
       483BC1               cmp      rax, rcx
       771E                 ja       SHORT G_M56801_IG13

G_M56801_IG09:
       48897DF0             mov      gword ptr [rbp-10H], rdi
       8975F8               mov      dword ptr [rbp-08H], esi
       8955FC               mov      dword ptr [rbp-04H], edx

G_M56801_IG10:
       488B45F0             mov      rax, gword ptr [rbp-10H]
       488B55F8             mov      rdx, qword ptr [rbp-08H]

G_M56801_IG11:
       488D6500             lea      rsp, [rbp]
       5D                   pop      rbp
       C3                   ret

G_M56801_IG12:
       E87EF3FFFF           call     ThrowHelper:ThrowArgumentOutOfRangeException()
       CC                   int3

G_M56801_IG13:
       E878F3FFFF           call     ThrowHelper:ThrowArgumentOutOfRangeException()
       CC                   int3

; Total bytes of code 105, prolog size 19 for method Program:PR(ref,int,int):struct
; ============================================================

@stephentoub stephentoub added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Mar 13, 2019
@stephentoub
Copy link
Member

Thanks. Given that this PR doesn't actually improve the generated code or resulting performance (and makes the code harder to read in the process), I'm going to close this. If in the future it becomes warranted, please feel free to open a new PR. Thanks.

@gfoidl gfoidl deleted the start_length_check branch March 15, 2019 15:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
* NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants