[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