[X86] `acc -= val < 0` converted to less efficient `acc += (val >> 7) & 1`

[dzaima]

The code:

#include <immintrin.h>
#include <stdint.h>

__m256i count_negative_values(__m256i* src, size_t n) {
    __m256i acc = _mm256_setzero_si256();

    for (size_t i = 0; i < n; i++) {
        __m256i c = _mm256_loadu_si256(src + i);
        __m256i m = _mm256_cmpgt_epi8(_mm256_setzero_si256(), c);
        acc = _mm256_sub_epi8(acc, m);
    }

    return acc;
}

with -O3 -march=haswell generates a sequence of vpsrlw …,7 + vpand + vpaddb for adding to the accumulator, whereas it could produce just vpcmpgtb + vpsubb. (if desired, in the unrolled case, it could, to add e.g. a,b,c,d to the accumulator without a long vpsubb chain, do acc -= (a+b)+(c+d))

https://godbolt.org/z/Wo7qE8MjG

[llvmbot]

@llvm/issue-subscribers-backend-x86

Author: dzaima (dzaima)

The code: ```c #include <immintrin.h> #include <stdint.h>

__m256i count_negative_values(__m256i* src, size_t n) {
__m256i acc = _mm256_setzero_si256();

for (size_t i = 0; i < n; i++) {
    __m256i c = _mm256_loadu_si256(src + i);
    __m256i m = _mm256_cmpgt_epi8(_mm256_setzero_si256(), c);
    acc = _mm256_sub_epi8(acc, m);
}

return acc;

}

with `-O3 -march=haswell` generates a sequence of `vpsrlw …,7` + `vpand` + `vpaddb` for adding to the accumulator, whereas it could produce just `vpcmpgtb` + `vpsubb`. (if desired, in the unrolled case, it could, to add e.g. `a`,`b`,`c`,`d` to the accumulator without a long `vpsubb` chain, do `acc -= (a+b)+(c+d)`)

https://godbolt.org/z/Wo7qE8MjG
</details>

[sharkautarch]

Looks like the instcombine pass is turning the vector compare + sign extension (seems that in llvm ir, vector compare produces a vector of 1-bit elements, which then has to be sign/zero-extended back to a vector of byte-elements) from _mm256_cmpgt_epi8(_mm256_setzero_si256(), c) into a simd bit shift:
https://godbolt.org/z/nTKKno936

I think the solution to this is to simply not instcombine vector cmp+sext patterns on x86, since pcmp on x86 behaves like llvm ir cmp+sext (produces all-ones for true and 0 for false)

[RKSimon]

Except this pattern is great for other vector types - its just because SSE/AVX doesn't have vXi8 shifts.

Also, InstCombine isn't (or shouldn't) be making target-specific decisions, but should canonicalize optimised patterns that the backends can efficiently handle - we'd be much better off just handling this either in then x86 backend or as a generic DAGCombiner fold.

[RKSimon]

#143106 caused an infinite loop in #143238