AArch64 SVE: Multiple ptrue instructions not merged

[MatzeB]

SVE code operating with different vector types has a tendency to produce multiple ptrue instructions.

For example I got this from an internal user:

#include <arm_sve.h>
svuint32_t getSveVec(const uint32_t* inputPtr) {
    svuint64_t vec = svld1uw_u64(svptrue_b64(), inputPtr);
    svuint32_t clzV1 = svclz_u32_x(svptrue_b32(), svreinterpret_u32_u64(vec));
    return clzV1;
}

Producing something like this:

$ clang++ -target aarch64-redhat-linux-gnu -march=armv9-a+sve2+fp16 -O3 -S -o - dup2.cpp
...
        ptrue   p0.d
        ld1w    { z0.d }, p0/z, [x0]
        ptrue   p0.s
        clz     z0.s, p0/m, z0.s
        ret

My understanding is that a ptrue p0.b would suffice here and in fact GCC is producing that code.

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: Matthias Braun (MatzeB)

SVE code operating with different vector types has a tendency to produce multiple `ptrue` instructions.

For example I got this from an internal user:

#include <arm_sve.h>
svuint32_t getSveVec(const uint32_t* inputPtr) {
    svuint64_t vec = svld1uw_u64(svptrue_b64(), inputPtr);
    svuint32_t clzV1 = svclz_u32_x(svptrue_b32(), svreinterpret_u32_u64(vec));
    return clzV1;
}

Producing something like this:

$ clang++ -target aarch64-redhat-linux-gnu -march=armv9-a+sve2+fp16 -O3 -S -o - dup2.cpp
...
        ptrue   p0.d
        ld1w    { z0.d }, p0/z, [x0]
        ptrue   p0.s
        clz     z0.s, p0/m, z0.s
        ret

My understanding is that a ptrue p0.b would suffice here and in fact GCC is producing that code.

[MatzeB]

(this mirrors meta T221893644)

[MatzeB]

Looking at this for a bit, I get the impression that this stems from SVE SelectionDAG modeling things with finegrained separate types like nxv4i1 / nxv2i1 etc. that then produce instructions like ptrue_s / ptrue_d. My impression though is that we can just as well use ptrue_b to produce any nxvXi1 values as all users of nxvXi1 just care about every n-th bit in the PPR register and ignore the other bits, so may just as well set them to 1 with ptrue_b.

[MatzeB]

Draft in #137042 Though I still see some remaining ptrue_X that could be ptrue_b that I haven't catched...

[davemgreen]

What would happen if someone wants a 0101010101010101 mask and is using a ptrue.h to generate it? (I think that is how they work).

[efriedma-quic]

An 0101010101010101 mask would have a different representation in the SelectionDAG, so it wouldn't match immAllOnesV.

[MatzeB]

@davemgreen I'm also trying to figure this out (still kinda new to aarch64 / SVE isel). My impression so far is that for an operation with an operand with nxv4i1 for the predicate at the selection dag level the question is moot: My impression was that it would only be used on operations say working on nxv4i32 vectors that then in turn would only read every 4th bit in the mask. That would mean all the bits in between would be "don't care" and we can set them to whatever we want for nxv4i1 values at the selection DAG level?

Or put another way if you actually want 0101010101010101 you would need an nxv64i1 value or so, which just isn't naturally occuring at the moment?

Please correct me if I'm wrong here...

[efriedma-quic]

That would mean all the bits in between would be "don't care" and we can set them to whatever we want for nxv4i1 values at the selection DAG level?

Right; we don't need to preserve the bits that aren't part of the type.

At the source level, the svptrue_b32() intrinsic has a return type of nxv16i1.

[MatzeB]

Indeed a function like

svbool_t bla() {
  return svptrue_b32();
}

appears to get miscompiled with my protype now. It ends up as %0 = tail call <vscale x 16 x i1> @llvm.aarch64.sve.convert.to.svbool.nxv4i1(<vscale x 4 x i1> splat (i1 true)) in IR and my impression is that convert.to.svbook.nxv4i1 is compiled to a NOP at the moment so it wouldn't explicitely set the "don't care" entries to "0" which it probably would have to do in my interpretation of what vscale x 4 x i1 means...

[efriedma-quic]

I think there's some code in SelectionDAG that's making assumptions which contradict your patch. See "isZeroingInactiveLanes".

[davemgreen]

There was a recent patch at the IR level to convert ptrue intrinsics to all-ones masks, implemented in #135016 by @MDevereau. Hopefully that didn't make it more difficult (or wrong). @sdesmalen-arm and @paulwalker-arm might know more about how to make this work nicely in DAG.

[paulwalker-arm]

Sorry, I tried and failed to respond to this properly so I'll look more closely next week. Until then I will just broadly say that we've typically tried to avoid doing this because it becomes tricky to code generate when you have to worry about "invisible bits". As Eli said, we've baked in some assumptions (e.g. a predicate splat will ensure "invisible bits" are zero) and have tried to keep the obscure cases to a minimum. We've also had performance issues relating to highly optimised ACLE code where the compiler erroneously generated ptrues that were not in the original source. This is why we have combines where we might look through predicate casting but we don't typically remove the casts themselves unless they are truly a NOP.

This is not to say the requested optimisation is not desirable but my hope was that it would be something we can do post ISEL when we don't really care about the semantic knowledge of IR based element types. Though, I admit to not giving this enough thought to determine how practical such a solution is.