Inlined ReadOnlySpan<char> parameter lacks string-like optimizations

[stephentoub]

Repro:
SharpLab

#nullable disable
using System;
using System.Runtime.CompilerServices;

public class C {
    
    static int M1() => DoWork1("a");
    static int M2() => DoWork2("a");
    
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    static int DoWork1(ReadOnlySpan<char> value)
    {
        if (value.Length == 1)
        {
            switch (value[0] | 0x20)
            {
                case 'a': return 1;
                case 'b': return 2;
                case 'c': return 4;
                case 'd': return 8;
                case 'e': return 16;
                case 'f': return 32;
            }
        }

        throw new Exception();
    }
    
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    static int DoWork2(string value)
    {
        if (value.Length == 1)
        {
            switch (value[0] | 0x20)
            {
                case 'a': return 1;
                case 'b': return 2;
                case 'c': return 4;
                case 'd': return 8;
                case 'e': return 16;
                case 'f': return 32;
            }
        }

        throw new Exception();
    }
}

M1 produces:

C.M1()
    L0000: push rsi
    L0001: sub rsp, 0x20
    L0005: mov rax, 0x23aa049dbd8
    L000f: mov rax, [rax]
    L0012: add rax, 0xc
    L0016: movzx eax, word ptr [rax]
    L0019: or eax, 0x20
    L001c: add eax, 0xffffff9f
    L001f: cmp eax, 5
    L0022: ja short L006a
    L0024: mov eax, eax
    L0026: lea rcx, [0x7ffcc3d200d8]
    L002d: mov ecx, [rcx+rax*4]
    L0030: lea rdx, [L0005]
    L0037: add rcx, rdx
    L003a: jmp rcx
    L003c: mov eax, 1
    L0041: jmp short L0064
    L0043: mov eax, 2
    L0048: jmp short L0064
    L004a: mov eax, 4
    L004f: jmp short L0064
    L0051: mov eax, 8
    L0056: jmp short L0064
    L0058: mov eax, 0x10
    L005d: jmp short L0064
    L005f: mov eax, 0x20
    L0064: add rsp, 0x20
    L0068: pop rsi
    L0069: ret
    L006a: mov rcx, 0x7ffce3cf2e38
    L0074: call 0x00007ffd4376f130
    L0079: mov rsi, rax
    L007c: mov rcx, rsi
    L007f: mov rax, 0x7ffce3d05c00
    L0089: call qword ptr [rax]
    L008b: mov rcx, rsi
    L008e: call 0x00007ffd4369fb10
    L0093: int3

whereas M2 produces:

C.M2()
    L0000: mov eax, 1
    L0005: ret

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Repro:
SharpLab

#nullable disable
using System;
using System.Runtime.CompilerServices;

public class C {
    
    static int M1() => DoWork1("a");
    static int M2() => DoWork2("a");
    
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    static int DoWork1(ReadOnlySpan<char> value)
    {
        if (value.Length == 1)
        {
            switch (value[0] | 0x20)
            {
                case 'a': return 1;
                case 'b': return 2;
                case 'c': return 4;
                case 'd': return 8;
                case 'e': return 16;
                case 'f': return 32;
            }
        }

        throw new Exception();
    }
    
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    static int DoWork2(string value)
    {
        if (value.Length == 1)
        {
            switch (value[0] | 0x20)
            {
                case 'a': return 1;
                case 'b': return 2;
                case 'c': return 4;
                case 'd': return 8;
                case 'e': return 16;
                case 'f': return 32;
            }
        }

        throw new Exception();
    }
}

M1 produces:

C.M1()
    L0000: push rsi
    L0001: sub rsp, 0x20
    L0005: mov rax, 0x23aa049dbd8
    L000f: mov rax, [rax]
    L0012: add rax, 0xc
    L0016: movzx eax, word ptr [rax]
    L0019: or eax, 0x20
    L001c: add eax, 0xffffff9f
    L001f: cmp eax, 5
    L0022: ja short L006a
    L0024: mov eax, eax
    L0026: lea rcx, [0x7ffcc3d200d8]
    L002d: mov ecx, [rcx+rax*4]
    L0030: lea rdx, [L0005]
    L0037: add rcx, rdx
    L003a: jmp rcx
    L003c: mov eax, 1
    L0041: jmp short L0064
    L0043: mov eax, 2
    L0048: jmp short L0064
    L004a: mov eax, 4
    L004f: jmp short L0064
    L0051: mov eax, 8
    L0056: jmp short L0064
    L0058: mov eax, 0x10
    L005d: jmp short L0064
    L005f: mov eax, 0x20
    L0064: add rsp, 0x20
    L0068: pop rsi
    L0069: ret
    L006a: mov rcx, 0x7ffce3cf2e38
    L0074: call 0x00007ffd4376f130
    L0079: mov rsi, rax
    L007c: mov rcx, rsi
    L007f: mov rax, 0x7ffce3d05c00
    L0089: call qword ptr [rax]
    L008b: mov rcx, rsi
    L008e: call 0x00007ffd4369fb10
    L0093: int3

whereas M2 produces:

C.M2()
    L0000: mov eax, 1
    L0005: ret

Author:	stephentoub
Assignees:	-
Labels:	tenet-performance, area-CodeGen-coreclr
Milestone:	-

[EgorBo]

Thanks, this one should be relatively easy to handle in VN. Might require a new JIT-EE api though, e.g.

bool getStringChar(CORINFO_OBJECT_HANDLE handle, int index, USHORT* value)

or a more general

bool getObjectContent(CORINFO_OBJECT_HANDLE handle, BYTE* buffer, int bufferSize)

Not sure it makes a lot of sense to have the more general one, I don't see other opportunities for it except the string case for now.

[EgorBo]

@jkotas @jakobbotsch would you recommend to have a more general API here?

We might also want to handle constant folding for

static ReadOnlyChar<byte> RVA = {1,2,3,4};

RVA[1]

but that one can't be unified with the frozen object api though..

[jkotas]

I do not see a need for a general getObjectContent. I think an API that just handles strings would be good enough. It may make sense to make it follow the same shape as the existing getStringLiteral (buffer + bufferSize + startIndex).

I assume that the existing getStringLiteral does not work here since you do not have the token anymore by the time you need to do this optimization. Is that right?

We might also want to handle constant folding for

Can the existing getReadonlyStaticFieldValue work for this case?

[EgorBo]

Can the existing getReadonlyStaticFieldValue work for this case?

Ah, right, good point. Although, it's currently tricky in JIT - it seems that we're losing FieldSeq info fairly early in this case (somewhere in assertprop)