This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Add intrinsics and patterns for logical predicate instructions
ClosedPublic

Authored by dancgr on Nov 27 2019, 12:44 PM.

Details

Summary

Add instrinics and patters for the following logical predicate instructions:

  • and, ands, bic, bics, eor, eors
  • sel
  • orr, orrs, orn, orns, nor, nors, nand, nads

Event Timeline

dancgr created this revision.Nov 27 2019, 12:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 27 2019, 12:44 PM
This revision is now accepted and ready to land.Nov 27 2019, 1:01 PM
sdesmalen added inline comments.Nov 28 2019, 5:17 AM
llvm/include/llvm/IR/IntrinsicsAArch64.td
1089

Is there a specific reason you're adding the _p? Most of the intrinsics that are predicated don't specify this, which kind of breaks with the convention with the other SVE intrinsics in this file. Note that we can represent an unpredicated and with existing selectiondag nodes so we wouldn't ever need an unpredicated int_aarch64_sve_and. It would be nice if you can change that for this and the other intrinsics before you commit the patch.

dancgr marked an inline comment as done.Nov 28 2019, 8:48 AM
dancgr added inline comments.
llvm/include/llvm/IR/IntrinsicsAArch64.td
1089

I added the _p because those are predicate to predicate operations, but we actually don't need the _p for the predicated predicate to predicate and predicated vector to vector instructions. The same intrinsic definition will work for both. I will be removing the _p before committing.

dancgr updated this revision to Diff 231544.Nov 29 2019, 7:45 AM
dancgr marked an inline comment as done.
  • [AArch64][SVE] Rename intrinsics to re-use logical SVE intrinsics.

Added intrinsics re-name before committing as suggested by @sdesmalen.

Other than the two nits, LGTM otherwise.

llvm/include/llvm/IR/IntrinsicsAArch64.td
1093

Can you also swap these two (bic and bic_pred)? That makes that naming consistent with the others.

llvm/test/CodeGen/AArch64/sve-pred-log.ll
51

nit: is this indentation on purpose?

dancgr marked 3 inline comments as done.Nov 29 2019, 11:48 AM
dancgr updated this revision to Diff 231571.Nov 29 2019, 11:49 AM
  • [AArch64][SVE] Change bic instrincis to comply with other predicated instructions
dancgr updated this revision to Diff 231572.Nov 29 2019, 11:58 AM
  • [AArch64][SVE] Remove trailing backslash from test
dancgr updated this revision to Diff 232182.Dec 4 2019, 11:07 AM
  • [AArch64][SVE] Rename intrinsics to re-use logical SVE intrinsics.
  • [AArch64][SVE] Change bic instrincis to comply with other predicated instructions
  • [AArch64][SVE] Remove trailing backslash from test
sdesmalen added inline comments.Dec 4 2019, 2:20 PM
llvm/include/llvm/IR/IntrinsicsAArch64.td
1092

nit: I'd suggest naming this _unpred to remove any ambiguity on what _base means.

llvm/test/CodeGen/AArch64/sve-pred-log.ll
88

These tests still have .pred in the name, but can probably be removed now that these tests have moved to sve-int-log-pred.ll ?