This is an archive of the discontinued LLVM Phabricator instance.

[ARM,MVE] Fix vreinterpretq in big-endian mode.
ClosedPublic

Authored by simon_tatham on Jan 31 2020, 7:16 AM.

Details

Summary

In big-endian MVE, the simple vector load/store instructions (i.e.
both contiguous and non-widening) don't all store the bytes of a
register to memory in the same order: it matters whether you did a
VSTRB.8, VSTRH.16 or VSTRW.32. Put another way, the in-register
formats of different vector types relate to each other in a different
way from the in-memory formats.

So, if you want to 'bitcast' or 'reinterpret' one vector type as
another, you have to carefully specify which you mean: did you want to
reinterpret the register format of one type as that of the other,
or the memory format?

The ACLE vreinterpretq intrinsics are specified to reinterpret the
register format. But I had implemented them as LLVM IR bitcast, which
is specified for all types as a reinterpretation of the memory format.
So a vreinterpretq intrinsic, applied to values already in registers,
would code-generate incorrectly if compiled big-endian: instead of
emitting no code, it would emit a vrev.

To fix this, I've introduced a new IR intrinsic to perform a
register-format reinterpretation: @llvm.arm.mve.vreinterpretq. It's
implemented by a trivial isel pattern that expects the input in an
MQPR register, and just returns it unchanged.

In the clang codegen, I only emit this new intrinsic where it's
actually needed: I prefer a bitcast wherever it will have the right
effect, because LLVM understands bitcasts better. So we still generate
bitcasts in little-endian mode, and even in big-endian when you're
casting between two vector types with the same lane size.

For testing, I've moved all the codegen tests of vreinterpretq out
into their own file, so that they can have a different set of RUN
lines to check both big- and little-endian.

Diff Detail

Event Timeline

simon_tatham created this revision.Jan 31 2020, 7:16 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 31 2020, 7:16 AM
dmgreen accepted this revision.Feb 2 2020, 7:58 AM

Big endian making things fun again.

VECTOR_REG_CAST looks useful for lowering too. LGTM

llvm/lib/Target/ARM/ARMInstrMVE.td
3958

Please add a comment, similar to this one.

This revision is now accepted and ready to land.Feb 2 2020, 7:58 AM
This revision was automatically updated to reflect the committed changes.