Suspicious code generation when unrolling+vectorising memset

[WanderingAura]

Minimal example:

#include <memory.h>

volatile char bytes[16];

int main(void) {

    for (int i = 0; i < 500; i++) {
        memset((void*)bytes, 0, 16);
    }
}

https://godbolt.org/z/5s9M5MvhK

options: -O3 -fno-unroll-loops

main:
        mov     eax, 500
        xorps   xmm0, xmm0
.LBB0_1:
        movaps  xmmword ptr [rip + bytes], xmm0
        dec     eax
        jne     .LBB0_1
        xor     eax, eax
        ret

bytes:
        .zero   16

options: -O3

main:
        mov     eax, 500
        xorps   xmm0, xmm0
.LBB0_1:
        movaps  xmmword ptr [rip + bytes], xmm0
        add     eax, -10
        jne     .LBB0_1
        xor     eax, eax
        ret

bytes:
        .zero   16

When compiled with -O3 -fno-unroll-loops, the generated assembly instructions is as expected. But with -O3 it seems to unroll the loop without copying the body of the loop 10 times, causing the vector instruction (the vectorised memset) to be executed 10x less than expected.

[nikic]

You are always setting the same memory to the same value, so it's correct for later memsets to be optimized out. The whole loop could be optimized to a single memset.

Note that you are casting away the volatile qualifier in the memset call, so volatile is irrelevant here, and does not force preservation of all redundant memsets.

[WanderingAura]

Thanks for the clarification on the volatile qualifier being cast away.

Though, doesn't that mean memset loops like the above should always get optimized into a single memset? Currently, the compiler seems to be a bit inconsistent. For example -O3 optimizes 50 memsets into a 1 memset (https://godbolt.org/z/af5138sc3) and 500 memsets into 50 memsets (as above).

It also seems dependent on the unroll number. For example on zen 4 it unrolls the loop 50 times so 500 memsets instead turn into 10. In my opinion an optimization like this being dependent on -march type is a bit odd.

[nikic]

The optimization happens as a result of unrolling, so it is affected by target-dependent heuristics. It would be legal to do it independently of unrolling by hoisting the memset out of the loop, it's just not implemented. It does work for a plain store, implemented here I believe:

llvm-project/llvm/lib/Transforms/Scalar/LICM.cpp

Line 1269 in 470f456

} else if (auto *SI = dyn_cast<StoreInst>(&I)) {

It could be extended to the memset case.