[WebAssembly] Autovectorisation to v128.bitselect not working on loops

[Photosounder]

When compiling to WebAssembly with clang using -Os -msimd128 I'm noticing something strange when trying to use functions that match SIMD ops (I do this so that I can write truly portable SIMD). When I use this unrolled function it autovectorises nicely to v128.bitselect:

static inline v128_t opFD52_v128_bitselect(v128_t a, v128_t b, v128_t c)
{
    a.u64[0] = a.u64[0] & c.u64[0] | b.u64[0] & ~c.u64[0];
    a.u64[1] = a.u64[1] & c.u64[1] | b.u64[1] & ~c.u64[1];
    return a;
}

But when it's written as a loop (which is obviously the form I prefer) it falls back to using scalar operations on i64 lanes:

static inline v128_t opFD52_v128_bitselect(v128_t a, v128_t b, v128_t c)
{
    for (int i=0; i<2; i++)
        a.u64[i] = a.u64[i] & c.u64[i] | b.u64[i] & ~c.u64[i];
    return a;
}

Other functions I tried are correctly autovectorised in loop form. See on Godbolt: https://godbolt.org/z/ovh7944xj

[llvmbot]

@llvm/issue-subscribers-backend-webassembly

Author: Michel Rouzic (Photosounder)

When compiling to WebAssembly with clang using `-Os -msimd128` I'm noticing something strange when trying to use functions that match SIMD ops (I do this so that I can write truly portable SIMD). When I use this unrolled function it autovectorises nicely to `v128.bitselect`:

static inline v128_t opFD52_v128_bitselect(v128_t a, v128_t b, v128_t c)
{
    a.u64[0] = a.u64[0] & c.u64[0] | b.u64[0] & ~c.u64[0];
    a.u64[1] = a.u64[1] & c.u64[1] | b.u64[1] & ~c.u64[1];
    return a;
}

But when it's written as a loop (which is obviously the form I prefer) it falls back to using scalar operations on i64 lanes:

static inline v128_t opFD52_v128_bitselect(v128_t a, v128_t b, v128_t c)
{
    for (int i=0; i<2; i++)
        a.u64[i] = a.u64[i] & c.u64[i] | b.u64[i] & ~c.u64[i];
    return a;
}

Other functions I tried are correctly autovectorised in loop form. See on Godbolt: https://godbolt.org/z/ovh7944xj

[GrumpyPigSkin]

This isn't an issue with WebAssembly. Using the command line args: -Rpass-missed=loop-vectorize -Rpass-analysis=loop-vectorize, we can see that the loop-vectorizer sees the loop and does not unroll it because the of the "unsafe" operations caused by the type punning of going from u32 -> u64. This then leaves behind scalar code which the SLP-vectorizer cannot make the same optimisations as the manually unrolled versions. To get around this, keep all the operations as the same type:

// this will lead to the same code as the manually unrolled variant.
static inline v128_t opFD52_v128_bitselect(v128_t a, v128_t b, v128_t c)
{
    for (int i=0; i<4; i++) // Operates on u32[4]
        a.u32[i] = (a.u32[i] & c.u32[i]) | (b.u32[i] & ~c.u32[i]);
    return a;
}

The new output:

simd_test:
        local.get       0
        local.get       1
        v128.load       0:p2align=3
        local.tee       2
        v128.const      -1443984, -1443984, -1443984, -1443984
        i32x4.add
        local.get       2
        v128.const      168311057, 168311057, 168311057, 168311057
        i32x4.mul
        local.get       2
        v128.const      10, 10, 10, 10
        i32x4.gt_s
        v128.bitselect
        v128.store      0:p2align=3
        end_function

Output with -Rpass-missed=loop-vectorize -Rpass-analysis=loop-vectorize on the original code:

<source>:20:5: remark: loop not vectorized: loop induction variable could not be identified [-Rpass-analysis=loop-vectorize]
<source>:21:18: remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
Unsafe indirect dependence. [-Rpass-analysis=loop-vectorize]
   21 |         a.u64[i] = a.u64[i] & c.u64[i] | b.u64[i] & ~c.u64[i];
      |                  ^
<source>:20:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
   20 |     for (int i=0; i<2; i++)
      |     ^
Compiler returned: 0

[Photosounder]

I'm not sure this explanation entirely makes sense, if you remove everything except the bitselect function and the v128_t declaration then it makes the same complaint, and if you define v128_t as typedef union { uint64_t u64[2]; } v128_t; it does it too. So in other words by keeping everything 64 bit-only it complains about something that it doesn't complain about with 32 bits, which is weird.

Also as a standalone function (without static inline) it additionally complains about:

Unsafe indirect dependence. Memory location is the same as accessed at example.c:12:20 [-Rpass-analysis=loop-vectorize]
   12 |         a.u64[i] = a.u64[i] & c.u64[i] | b.u64[i] & ~c.u64[i];

Which is weird, because I can't see anything unsafe about this, and neither does the compiler with the 32 bit version. When storing the results to a new vector by doing r.u64[i] = ... it still complains about "value that could not be identified as reduction is used outside the loop" and "loop induction variable could not be identified" which is weird. I'm fairly confident that the compiler is wrong to see a problem there.

Besides I'd much rather keep it as 64 bits because this function also serves as a non-SIMD fallback.

[GrumpyPigSkin]

True, and when compiling for -O1/2/3/z there are no complaints and it produces code as you would expect, there are only unsafe complaints when compiling for -Os. Naturally the vectorisation huristics will be slightly different from to another, but I am not sure why it would complain about unsafe dependent memory operations in loop for -Os only.