[PATCH][AArch64-BE] Be careful when replacing a BUILD_VECTOR with a MOVI

Tim Northover t.p.northover at gmail.com
Fri Jul 18 08:30:06 PDT 2014


Hi James,

On 17 July 2014 18:10, James Molloy <james.molloy at arm.com> wrote:
> Can I ping this please?

I'm very uneasy about this one. It looks like it relies on the fact
that the output of lowering a resolved BUILD_VECTOR is basically "(VT
(bitconvert (f64 INST)))", which doesn't seem to be true in general.
I'd have put the underlying flaw in the lax attitude to casting
demonstrated by the *users* of this function, and said
"0x0000ffff0000ffff" was an entirely reasonable output for <-1, 0, -1,
0> given how we're representing vectors.

At the very least I think more paths (with different ModImmTypes, both
64 and 128-bit) should be tested and we should clearly document what
the correct interpretation of the CnstBits and UndefBits outputs of
resolveBUILD_VECTOR is.

There are some other fairly minor points I spotted along the way, but
they may be completely irrelevant now (see end).

Cheers.

Tim.

+      APInt NewCnstBits(64, 0), NewUndefBits(64, 0);

This function seems to be used in contexts where 128-bit vectors are permitted.

+      APInt Mask(64, (1ULL << Sz) - 1);

I think this is undefined behaviour if Sz == 64. There's
APInt::getAllOnesValue or APInt::getLowBitsSet instead.



More information about the llvm-commits mailing list