Suboptimal register use on x86-64

[tavianator]

Saw this while looking at #128441, but this seems like a more general issue. This C code

int foo(int *a, int *b, int size) {
    int ret = 0;

    for (int i = 0; i < size; ++i) {
        int diff = a[i] ^ b[i];
        ret += diff;
        if (!diff) {
            break;
        }
    }

    return ret;
}

compiles at all optimization levels to:

foo:
        test    edx, edx
        jle     .LBB0_1
        mov     ecx, edx
        dec     rcx
        xor     edx, edx
        xor     eax, eax
.LBB0_3:
        mov     r8d, eax
        mov     r9d, dword ptr [rdi + 4*rdx]
        mov     r10d, dword ptr [rsi + 4*rdx]
        mov     eax, r10d
        xor     eax, r9d
        add     eax, r8d
        xor     r10d, r9d
        je      .LBB0_5
        lea     r8, [rdx + 1]
        cmp     rcx, rdx
        mov     rdx, r8
        jne     .LBB0_3
.LBB0_5:
        ret
.LBB0_1:
        xor     eax, eax
        ret

It keeps moving the accumulator from eax to r8d and back. It also materializes the xor result twice when it doesn't need to. Ideally I think it would look more like this:

.LBB0_3:
        mov     r8d, dword ptr [rdi + 4*rdx]
        xor     r8d, dword ptr [rsi + 4*rdx]
        je      .LBB0_5
        add     eax, r8d
        inc     edx
        cmp     ecx, edx
        jne     .LBB0_3

[llvmbot]

@llvm/issue-subscribers-backend-x86

Author: Tavian Barnes (tavianator)

Saw this while looking at https://github.com//issues/128441, but this seems like a more general issue. This C code

int foo(int *a, int *b, int size) {
    int ret = 0;

    for (int i = 0; i < size; ++i) {
        int diff = a[i] ^ b[i];
        ret += diff;
        if (!diff) {
            break;
        }
    }

    return ret;
}

compiles at all optimization levels to:

foo:
        test    edx, edx
        jle     .LBB0_1
        mov     ecx, edx
        dec     rcx
        xor     edx, edx
        xor     eax, eax
.LBB0_3:
        mov     r8d, eax
        mov     r9d, dword ptr [rdi + 4*rdx]
        mov     r10d, dword ptr [rsi + 4*rdx]
        mov     eax, r10d
        xor     eax, r9d
        add     eax, r8d
        xor     r10d, r9d
        je      .LBB0_5
        lea     r8, [rdx + 1]
        cmp     rcx, rdx
        mov     rdx, r8
        jne     .LBB0_3
.LBB0_5:
        ret
.LBB0_1:
        xor     eax, eax
        ret

It keeps moving the accumulator from eax to r8d and back. It also materializes the xor result twice when it doesn't need to. Ideally I think it would look more like this:

.LBB0_3:
        mov     r8d, dword ptr [rdi + 4*rdx]
        xor     r8d, dword ptr [rsi + 4*rdx]
        je      .LBB0_5
        add     eax, r8d
        inc     edx
        cmp     ecx, edx
        jne     .LBB0_3

[phoebewang]

This is similar to #128441, we cannot tell the relationship between the producer and potential consumer of EFLAGS to the compiler unless we introduce such a builtin. The moving of registers is due to InstCombine prefers recomputing to using the result. I guess this is to reduce data dependency.
https://godbolt.org/z/nvnGfh61E
https://godbolt.org/z/f8YvvqoYr

[tavianator]

Yeah fair enough, but even this would be an improvement:

.LBB0_3:
        mov     r9d, dword ptr [rdi + 4*rdx]
        mov     r10d, dword ptr [rsi + 4*rdx]
        mov     r8d, r10d
        xor     r8d, r9d
        add     eax, r8d
        xor     r10d, r9d
        je      .LBB0_5

[phoebewang]

What's the improvement here? It's the same as generated code. The rest is to handle the loop count.

[tavianator]

What's the improvement here? It's the same as generated code. The rest is to handle the loop count.

Just saving one mov:

 .LBB0_3:
-        mov     r8d, eax
         mov     r9d, dword ptr [rdi + 4*rdx]
         mov     r10d, dword ptr [rsi + 4*rdx]
-        mov     eax, r10d
-        xor     eax, r9d
+        mov     r8d, r10d
+        xor     r8d, r9d
         add     eax, r8d
         xor     r10d, r9d
         je      .LBB0_5

[phoebewang]

What's the improvement here? It's the same as generated code. The rest is to handle the loop count.

Just saving one mov:

.LBB0_3:

•    mov     r8d, eax
     mov     r9d, dword ptr [rdi + 4*rdx]
     mov     r10d, dword ptr [rsi + 4*rdx]
  

•    mov     eax, r10d
  

•    xor     eax, r9d
  

•    mov     r8d, r10d
  

•    xor     r8d, r9d
     add     eax, r8d
     xor     r10d, r9d
     je      .LBB0_5
  

I see. I think this is due to we run out of free registers. The mov looks like a reg2reg spill to me. We will get the expected result if we can save one register by ndd: https://godbolt.org/z/64EKavPj9