Page MenuHomePhabricator

[NEON] Support vldNq intrinsics in AArch32 (LLVM part)

Authored by kosarev on Jun 21 2018, 8:58 AM.



This patch adds support for the q versions of the dup (load-to-all-lanes) NEON intrinsics, such as vld2q_dup_f16() for example.

Currently, non-q versions of the dup intrinsics are implemented in clang by generating IR that first loads the elements of the structure into the first lane with the lane (to-single-lane) intrinsics, and then propagating it other lanes. There are at least two problems with this approach. First, there are no double-spaced to-single-lane byte-element instructions. For example, there is no such instruction as 'vld2.8 { d0[0], d2[0] }, [r0]'. That means we cannot rely on the to-single-lane intrinsics and instructions to implement the q versions of the dup intrinsics. Note that to-all-lanes instructions do support all sizes of data items, including bytes.

The second problem with the current approach is that we need a separate vdup instruction to propagate the structure to each lane. So for vld4q_dup_f16() we would need four vdup instructions in addition to the initial vld instruction.

This patch introduces dup LLVM intrinsics and reworks handling of the currently supported (non-q) NEON dup intrinsics to expand them into those LLVM intrinsics, thus eliminating the need for using to-single-lane intrinsics and instructions.

Additionally, this patch adds support for u64 and s64 dup NEON intrinsics. These are marked as Arch64-only in the ARM NEON Reference, but it seems there are no reasons to not support them in AArch32 mode. Please correct, if that is wrong.

That's what we generate with this patch applied:

  vld2.16 {d0[], d2[]}, [r0]
  vld2.16 {d1[], d3[]}, [r0]

  vld3.16 {d0[], d2[], d4[]}, [r0]
  vld3.16 {d1[], d3[], d5[]}, [r0]

  vld4.16 {d0[], d2[], d4[], d6[]}, [r0]
  vld4.16 {d1[], d3[], d5[], d7[]}, [r0]

Diff Detail


Event Timeline

kosarev created this revision.Jun 21 2018, 8:58 AM
SjoerdMeijer added inline comments.Jun 27 2018, 1:11 AM
64 ↗(On Diff #152313)

Looks like "llvm.arm.neon.vld4dup.v2i64.p0i8" is not tested?

kosarev added inline comments.Jun 27 2018, 1:34 AM
64 ↗(On Diff #152313)

The initial plan was to support q versions of the 64-bit intrinsics, but then it became clear that would require some special code. Will remove these declarations on commit.

SjoerdMeijer accepted this revision.Jun 27 2018, 1:40 AM

Looks OK to me

This revision is now accepted and ready to land.Jun 27 2018, 1:40 AM
dnsampaio reopened this revision.Jul 3 2018, 4:44 AM
This comment was removed by dnsampaio.
This revision is now accepted and ready to land.Jul 3 2018, 4:44 AM
dnsampaio requested changes to this revision.Jul 3 2018, 4:48 AM

Please add the new intrinsics to the target specific combine function of VLDUP NEON load/store intrinsics
ARMISelLowering.cpp, line 11477. The switch dies on llvm_unreachable.

This revision now requires changes to proceed.Jul 3 2018, 4:48 AM

OK, I'm on it. Thanks.

dnsampaio accepted this revision.Jul 4 2018, 8:33 AM

All good.

This revision is now accepted and ready to land.Jul 4 2018, 8:33 AM
kosarev closed this revision.Jul 5 2018, 2:05 AM

D48920 is landed.