aarch64 splits one NEON shift into two (missed optimization)

[neldredge]

Given the following aarch64 NEON intrinsic code:

#include <arm_neon.h>
void vectorized(const uint8_t* pCSI2, uint8_t* pBE, const uint8_t* pCSI2LineEnd)
{
    while (pCSI2 < pCSI2LineEnd) {
        uint8x16x3_t in = vld3q_u8(pCSI2);
        uint8x16x3_t out;
        out.val[0] = in.val[0];
        out.val[1] = vorrq_u8(vshlq_n_u8(in.val[2], 4), vshrq_n_u8(in.val[1], 4));
        out.val[2] = vorrq_u8(vshlq_n_u8(in.val[1], 4), vshrq_n_u8(in.val[2], 4));
        vst3q_u8(pBE, out);
        pCSI2 += 48;
        pBE += 48;
    }
}

For the vshrq_n_u8, instead of the obvious ushr v4.16b, v1.16b, #4, clang emits

ushr    v4.16b, v1.16b, #1
ushr    v4.16b, v4.16b, #3

This does not happen with a function consisting only of vshrq_n_u8(x, 4), so it apparently depends on the context somehow.

Try on godbolt

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

[davemgreen]

This gets transformed into a funnel shift, which we still need better vector lowering for. They should be a ushr;sli if I haven't got that wrong.
(And maybe we need to combine shifts together in any case, which is presumably missing from AArch64ISD::VSHL.

Using vsli intrinsics should make that happen without transforming into a funnel shift: https://godbolt.org/z/oPMsqobKr

[efriedma-quic]

The double shift is coming from TargetLowering::expandFunnelShift, which somehow doesn't recognize the shift amount as a constant.

[RKSimon]

The double shift is coming from TargetLowering::expandFunnelShift, which somehow doesn't recognize the shift amount as a constant.

Is this related to da0f248 ? Widening creates undef elements and uniform constants are lost when we constant fold undef to zero.

[efriedma-quic]

Widening creates undef elements

I didn't see any undef? I think the pattern-matching just doesn't like BUILD_VECTORS where the operand type is legalized to a wider type than the result type.

[c-rhodes]

fixed on trunk, suspect by #123597 but I've not confirmed this