JIT generates redundant mov for inline ++i (pre-increment)

[rameel]

https://godbolt.org/z/aYMq87E9G

// Simplified repro
public static int N(ReadOnlySpan<char> s, int i)
{
    while (true)
    {
        if ((uint)++i >= (uint)s.Length)
            break;

        if (s[i] == '?')
            return i;
    }

    return -1;
}

Generated ASM:

lea      eax, [rdx+0x01]
mov      edx, eax          ; redundant
cmp      edx, esi

With a separate increment:

++i;
if ((uint)i >= (uint)s.Length) 
    break;

Generated ASM:

inc      edx
cmp      edx, esi

.NET 8.0 behavior: both variants generate identical inc; cmp code.

Issue:

• Inline ++i adds an extra mov, increasing register pressure
• Causes spills in register-constrained methods
• Larger binary size

[dotnet-policy-service]

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

[BoyBaykiller]

This is not what this issue is talking about but why is this loop cloned? There are no bounds checks. Maybe I am missunderstanding what loop cloning does. It wasn't cloned previously. @AndyAyersMS
We get:

G_M23659_IG02:  ;; offset=0x0000
       mov      rax, bword ptr [rcx]
       mov      ecx, dword ptr [rcx+0x08]
       cmp      edx, ecx
       jae      SHORT G_M23659_IG08
       align    [0 bytes for IG03]
						;; size=10 bbWeight=1 PerfScore 5.25
G_M23659_IG03:  ;; offset=0x000A
       inc      edx
       cmp      edx, ecx
       jae      SHORT G_M23659_IG09
						;; size=6 bbWeight=7.92 PerfScore 11.88
G_M23659_IG04:  ;; offset=0x0010
       mov      r8d, edx
       cmp      word  ptr [rax+2*r8], 63
       jne      SHORT G_M23659_IG03
						;; size=11 bbWeight=3.96 PerfScore 12.87
G_M23659_IG05:  ;; offset=0x001B
       mov      eax, edx
						;; size=2 bbWeight=0.50 PerfScore 0.12
G_M23659_IG06:  ;; offset=0x001D
       ret      
						;; size=1 bbWeight=0.50 PerfScore 0.50
G_M23659_IG07:  ;; offset=0x001E
       mov      r8d, edx
       cmp      word  ptr [rax+2*r8], 63
       je       SHORT G_M23659_IG05
						;; size=11 bbWeight=0.04 PerfScore 0.13
G_M23659_IG08:  ;; offset=0x0029
       inc      edx
       cmp      edx, ecx
       jb       SHORT G_M23659_IG07
						;; size=6 bbWeight=0.08 PerfScore 0.12
G_M23659_IG09:  ;; offset=0x002F
       mov      eax, -1
						;; size=5 bbWeight=0.50 PerfScore 0.12
G_M23659_IG10:  ;; offset=0x0034
       ret      
						;; size=1 bbWeight=0.50 PerfScore 0.50

While I would have expected this (with DOTNET_JitCloneLoops=0 to repro):

G_M23659_IG02:  ;; offset=0x0000
       mov      rax, bword ptr [rcx]
       mov      ecx, dword ptr [rcx+0x08]
       mov      edx, edx
       align    [0 bytes for IG03]
						;; size=8 bbWeight=1 PerfScore 4.25
G_M23659_IG03:  ;; offset=0x0008
       inc      edx
       cmp      edx, ecx
       jae      SHORT G_M23659_IG07
						;; size=6 bbWeight=8 PerfScore 12.00
G_M23659_IG04:  ;; offset=0x000E
       cmp      word  ptr [rax+2*rdx], 63
       jne      SHORT G_M23659_IG03
						;; size=7 bbWeight=4 PerfScore 12.00
G_M23659_IG05:  ;; offset=0x0015
       mov      eax, edx
						;; size=2 bbWeight=0.50 PerfScore 0.12
G_M23659_IG06:  ;; offset=0x0017
       ret      
						;; size=1 bbWeight=0.50 PerfScore 0.50
G_M23659_IG07:  ;; offset=0x0018
       mov      eax, -1
						;; size=5 bbWeight=0.50 PerfScore 0.12
G_M23659_IG08:  ;; offset=0x001D
       ret      
						;; size=1 bbWeight=0.50 PerfScore 0.50

[AndyAyersMS]

My guess is that we cloned to get rid of the bounds check and then were also able to get rid of the bounds check in the slow loop via other means.

Often when that happens we're also able to optimize away the slow loop so we end up more or less with your after picture. But evidently not so here.

Feel free to investigate further. I could be wrong.

[BoyBaykiller]

Would it make sense to first do all rangechecks opts and stuff and only at the very end if we still have BOUNDS_CHECK_Rng clone the loop.