CQ: Manually hoisted Vector{256,128} constant is reloaded inside the loop (sometimes)

[gfoidl]

Repro (.NET 7, applies to both Vector128 and Vector256):

static void Foo(ref byte src, nuint length, ref byte dest)
{
    nuint idx = 0;

    // Manually hoisted out of the loop
    Vector128<byte> mask = Vector128.Create((byte)0x7F);

    if (length >= (uint)Vector128<byte>.Count)
    {
        nuint vecEnd = length - (uint)Vector128<byte>.Count;

        do
        {
            Vector128<byte> vec = Vector128.LoadUnsafe(ref src, idx);
            vec &= mask;     // use of hoisted constant

            vec.StoreUnsafe(ref dest, idx);

            idx += (uint)Vector128<byte>.Count;
        } while (idx <= vecEnd);
    }

    // Remainder not relevant
}

; Assembly listing for method Program:<<Main>$>g__Foo|0_0(byref,long,byref)
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; Tier-1 compilation
; optimized code
; rsp based frame
; fully interruptible
; No PGO data

G_M000_IG01:                ;; offset=0000H
       C5F877               vzeroupper

G_M000_IG02:                ;; offset=0003H
       33C0                 xor      eax, eax
       4883FA10             cmp      rdx, 16
       7223                 jb       SHORT G_M000_IG05

G_M000_IG03:                ;; offset=000BH
       4883C2F0             add      rdx, -16
       90                   align    [1 bytes for IG04]

G_M000_IG04:                ;; offset=0010H
       C5FA6F0401           vmovdqu  xmm0, xmmword ptr [rcx+rax]
       C5F9DB0513000000     vpand    xmm0, xmm0, xmmword ptr [reloc @RWD00]     ; should be kept hoisted out of the loop
       C4C17A7F0400         vmovdqu  xmmword ptr [r8+rax], xmm0
       4883C010             add      rax, 16
       483BC2               cmp      rax, rdx
       76E4                 jbe      SHORT G_M000_IG04

G_M000_IG05:                ;; offset=002CH
       C3                   ret

RWD00   dq      7F7F7F7F7F7F7F7Fh, 7F7F7F7F7F7F7F7Fh

; Total bytes of code 45

For this simple case the workaround is to just define the vector-constant inline, then the JIT will hoist the variable:

- vec &= mask;     // use of hoisted constant
+ vec &= Vector128.Create((byte)0x7F);    // here the JIT will hoist the constant

G_M000_IG03:                ;; offset=000BH
       4883C2F0             add      rdx, -16
       C4E179100528000000   vmovupd  xmm0, xmmword ptr [reloc @RWD00]	; hoisted by JIT
       0F1F840000000000     align    [8 bytes for IG04]

G_M000_IG04:                ;; offset=0020H
       C5F9DB0C01           vpand    xmm1, xmm0, xmmword ptr [rcx+rax]
       C4C17A7F0C00         vmovdqu  xmmword ptr [r8+rax], xmm1
       4883C010             add      rax, 16
       483BC2               cmp      rax, rdx
       76EC                 jbe      SHORT G_M000_IG04

Hower it's not always that easy. If the loop body is more complex (like e.g. having some method calls that will be inlined or for base64 encoding) then the constant vector probably isn't hoisted by the JIT. Hoisting or not seems to be a bit sensitive to actual code.

Of course one can check codegen to see what happens, but I'd like the JIT to keep manually hoisted constant outside the loop.
Porting code from say .NET 5 to .NET 7 has potential perf-traps, as constants that was kept hoisted in .NET 5 may get reloaded inside the loop with .NET 7. Even if this is known by inspecting the machine code, this would lead to #ifdefs to have optimal codegen for several targets.

---

I think it's the same codegen-issue if the lower lane of the Vector256 should be re-used as Vector128. The the Vector128 may be reloaded from memory instead of just using the xmm register of the ymm register.
E.g.

static void Foo(ref byte src, ref byte dest)
{
    Vector256<byte> mask256 = Vector256.Create((byte)0x7F);

    Vector256<byte> vec256 = Vector256.LoadUnsafe(ref src);
    vec256 &= mask256;
    vec256.StoreUnsafe(ref dest);

    Vector128<byte> mask128 = mask256.GetLower();    // reuse the already present vector instead of reloading from memory

    Vector128<byte> vec128 = Vector128.LoadUnsafe(ref src);
    vec128 &= mask128;
    vec128.StoreUnsafe(ref dest);
}

; Assembly listing for method Program:<<Main>$>g__Foo|0_0(byref,byref)
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; Tier-1 compilation
; optimized code
; rsp based frame
; partially interruptible
; No PGO data

G_M000_IG01:                ;; offset=0000H
       C5F877               vzeroupper

G_M000_IG02:                ;; offset=0003H
       C5FE6F01             vmovdqu  ymm0, ymmword ptr[rcx]
       C5FDDB0531000000     vpand    ymm0, ymm0, ymmword ptr[reloc @RWD00]
       C5FE7F02             vmovdqu  ymmword ptr[rdx], ymm0
       C5FE6F0525000000     vmovdqu  ymm0, ymmword ptr[reloc @RWD00]    ; instead of reuse like written in C# it's reloaded
       C5F9DB01             vpand    xmm0, xmm0, xmmword ptr [rcx]
       C5FA7F02             vmovdqu  xmmword ptr [rdx], xmm0

G_M000_IG03:                ;; offset=0023H
       C5F877               vzeroupper
       C3                   ret

RWD00   dq      7F7F7F7F7F7F7F7Fh, 7F7F7F7F7F7F7F7Fh, 7F7F7F7F7F7F7F7Fh, 7F7F7F7F7F7F7F7Fh

; Total bytes of code 39

Here I'd expect code similar to

+      C5FE6F0528000000     vmovdqu  ymm1, ymmword ptr[reloc @RWD00]
G_M000_IG02:                ;; offset=0003H
       C5FE6F01             vmovdqu  ymm0, ymmword ptr[rcx]
-      C5FDDB0531000000     vpand    ymm0, ymm0, ymmword ptr[reloc @RWD00]
+      C5FDDB0531000000     vpand    ymm0, ymm0, ymm1
       C5FE7F02             vmovdqu  ymmword ptr[rdx], ymm0
-      C5FE6F0525000000     vmovdqu  ymm0, ymmword ptr[reloc @RWD00]
       C5F9DB01             vpand    xmm0, xmm1, xmmword ptr [rcx]
       C5FA7F02             vmovdqu  xmmword ptr [rdx], xmm0

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

Issue Details

---

Repro (.NET 7, applies to both Vector128 and Vector256):

static void Foo(ref byte src, nuint length, ref byte dest)
{
    nuint idx = 0;

    // Manually hoisted out of the loop
    Vector128<byte> mask = Vector128.Create((byte)0x7F);

    if (length >= (uint)Vector128<byte>.Count)
    {
        nuint vecEnd = length - (uint)Vector128<byte>.Count;

        do
        {
            Vector128<byte> vec = Vector128.LoadUnsafe(ref src, idx);
            vec &= mask;     // use of hoisted constant

            vec.StoreUnsafe(ref dest, idx);

            idx += (uint)Vector128<byte>.Count;
        } while (idx <= vecEnd);
    }

    // Remainder not relevant
}

; Assembly listing for method Program:<<Main>$>g__Foo|0_0(byref,long,byref)
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; Tier-1 compilation
; optimized code
; rsp based frame
; fully interruptible
; No PGO data

G_M000_IG01:                ;; offset=0000H
       C5F877               vzeroupper

G_M000_IG02:                ;; offset=0003H
       33C0                 xor      eax, eax
       4883FA10             cmp      rdx, 16
       7223                 jb       SHORT G_M000_IG05

G_M000_IG03:                ;; offset=000BH
       4883C2F0             add      rdx, -16
       90                   align    [1 bytes for IG04]

G_M000_IG04:                ;; offset=0010H
       C5FA6F0401           vmovdqu  xmm0, xmmword ptr [rcx+rax]
       C5F9DB0513000000     vpand    xmm0, xmm0, xmmword ptr [reloc @RWD00]     ; should be kept hoisted out of the loop
       C4C17A7F0400         vmovdqu  xmmword ptr [r8+rax], xmm0
       4883C010             add      rax, 16
       483BC2               cmp      rax, rdx
       76E4                 jbe      SHORT G_M000_IG04

G_M000_IG05:                ;; offset=002CH
       C3                   ret

RWD00   dq      7F7F7F7F7F7F7F7Fh, 7F7F7F7F7F7F7F7Fh

; Total bytes of code 45

For this simple case the workaround is to just define the vector-constant inline, then the JIT will hoist the variable:

- vec &= mask;     // use of hoisted constant
+ vec &= Vector128.Create((byte)0x7F);    // here the JIT will hoist the constant

G_M000_IG03:                ;; offset=000BH
       4883C2F0             add      rdx, -16
       C4E179100528000000   vmovupd  xmm0, xmmword ptr [reloc @RWD00]	; hoisted by JIT
       0F1F840000000000     align    [8 bytes for IG04]

G_M000_IG04:                ;; offset=0020H
       C5F9DB0C01           vpand    xmm1, xmm0, xmmword ptr [rcx+rax]
       C4C17A7F0C00         vmovdqu  xmmword ptr [r8+rax], xmm1
       4883C010             add      rax, 16
       483BC2               cmp      rax, rdx
       76EC                 jbe      SHORT G_M000_IG04

Hower it's not always that easy. If the loop body is more complex (like e.g. having some method calls that will be inlined or for base64 encoding) then the constant vector probably isn't hoisted by the JIT. Hoisting or not seems to be a bit sensitive to actual code.

Of course one can check codegen to see what happens, but I'd like the JIT to keep manually hoisted constant outside the loop.
Porting code from say .NET 5 to .NET 7 has potential perf-traps, as constants that was kept hoisted in .NET 5 may get reloaded inside the loop with .NET 7. Even if this is known by inspecting the machine code, this would lead to #ifdefs to have optimal codegen for several targets.

---

I think it's the same codegen-issue if the lower lane of the Vector256 should be re-used as Vector128. The the Vector128 may be reloaded from memory instead of just using the xmm register of the ymm register.
E.g.

static void Foo(ref byte src, ref byte dest)
{
    Vector256<byte> mask256 = Vector256.Create((byte)0x7F);

    Vector256<byte> vec256 = Vector256.LoadUnsafe(ref src);
    vec256 &= mask256;
    vec256.StoreUnsafe(ref dest);

    Vector128<byte> mask128 = mask256.GetLower();    // reuse the already present vector instead of reloading from memory

    Vector128<byte> vec128 = Vector128.LoadUnsafe(ref src);
    vec128 &= mask128;
    vec128.StoreUnsafe(ref dest);
}

; Assembly listing for method Program:<<Main>$>g__Foo|0_0(byref,byref)
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; Tier-1 compilation
; optimized code
; rsp based frame
; partially interruptible
; No PGO data

G_M000_IG01:                ;; offset=0000H
       C5F877               vzeroupper

G_M000_IG02:                ;; offset=0003H
       C5FE6F01             vmovdqu  ymm0, ymmword ptr[rcx]
       C5FDDB0531000000     vpand    ymm0, ymm0, ymmword ptr[reloc @RWD00]
       C5FE7F02             vmovdqu  ymmword ptr[rdx], ymm0
       C5FE6F0525000000     vmovdqu  ymm0, ymmword ptr[reloc @RWD00]    ; instead of reuse like written in C# it's reloaded
       C5F9DB01             vpand    xmm0, xmm0, xmmword ptr [rcx]
       C5FA7F02             vmovdqu  xmmword ptr [rdx], xmm0

G_M000_IG03:                ;; offset=0023H
       C5F877               vzeroupper
       C3                   ret

RWD00   dq      7F7F7F7F7F7F7F7Fh, 7F7F7F7F7F7F7F7Fh, 7F7F7F7F7F7F7F7Fh, 7F7F7F7F7F7F7F7Fh

; Total bytes of code 39

Here I'd expect code similar to

+      C5FE6F0528000000     vmovdqu  ymm1, ymmword ptr[reloc @RWD00]
G_M000_IG02:                ;; offset=0003H
       C5FE6F01             vmovdqu  ymm0, ymmword ptr[rcx]
-      C5FDDB0531000000     vpand    ymm0, ymm0, ymmword ptr[reloc @RWD00]
+      C5FDDB0531000000     vpand    ymm0, ymm0, ymm1
       C5FE7F02             vmovdqu  ymmword ptr[rdx], ymm0
-      C5FE6F0525000000     vmovdqu  ymm0, ymmword ptr[reloc @RWD00]
       C5F9DB01             vpand    xmm0, xmm1, xmmword ptr [rcx]
       C5FA7F02             vmovdqu  xmmword ptr [rdx], xmm0

Author:	gfoidl
Assignees:	-
Labels:	area-CodeGen-coreclr, untriaged
Milestone:	-

[EgorBo]

looks like another form of #70355 I can take a look since I fixed it for obvious cases in #70378 but the heuristics need to be adjusted

[Daniel-Svensson]

An alternative workaround when doing manual hoisting that seems to work is to save the constant in a static readonly variable and load that one instead of the constant (but it has it's own downsides).