[PATCH] [AArch64 - BE] BUILD_VECTOR lane order is reversed in big-endian mode

Tim Northover t.p.northover at gmail.com
Fri Aug 29 10:30:56 PDT 2014


Hi Asiri,

I'm afraid I still think that this isn't fixing the real issue, which is the dodgy casting that goes on *after* resolveBuildVector. Putting the change there seems like it would be trying to make two bugs cancel out into a feature (though I can't find any currently incorrect code that results).

  - It makes the resolveBuildVector's output incompatible with its fundamental building-block, isConstantSplat.
  - It means that CnstBits no longer represent the in-register value directly: "CnstBits & 1" no longer checks if the value would be even, for example.
  - We now end up passing a big-endian value into endian-agnostic isAdvSIMDModImmTypeN functions.

We bodge that last point up with some REVs afterwards, but that only works in the direct MOVI case, I think. For example:

    @vec = global <4 x i16> <i16 1234, i16 5678, i16 9101, i16 1121>

    define i16 @foo() {
      %in = load <4 x i16>* @vec
      %res_vec = and <4 x i16> %in, <i16 65400, i16 65535, i16 65400, i16 65535>
      %elt = extractelement <4 x i16> %res_vec, i16 0
      ret i16 %elt
    }

This currently produces:

        ld1     { v0.4h }, [x8]
        bic     v0.2s, #0x87
        rev32   v0.4h, v0.4h
        umov    w0, v0.h[0]

where that REV32 is highly suspect. But after your patch it produces:

        ld1     { v0.4h }, [x8]
        bic     v0.2s, #0x87, lsl #16
        rev32   v0.4h, v0.4h
        umov    w0, v0.h[0]

where both the BIC and the REV32 are wrong, and don't cancel each other out.

Do my concerns make more sense now?

Cheers.

Tim.

http://reviews.llvm.org/D5097






More information about the llvm-commits mailing list