Page MenuHomePhabricator

[AArch64][SVE] Add patterns for some integer vector instructions
ClosedPublic

Authored by dancgr on Oct 17 2019, 11:47 AM.

Details

Summary

This is another follow up to https://reviews.llvm.org/D68098.

Add pattern matching for SVE vector instructions:

  • add, sub, and, or, xor instructions
  • sqadd, uqadd, sqsub, uqsub target-independent intrinsics
  • bic intrinsics
  • predicated add, sub, subr intrinsics

Diff Detail

Repository
rL LLVM

Event Timeline

dancgr created this revision.Oct 17 2019, 11:47 AM
dancgr edited the summary of this revision. (Show Details)Oct 17 2019, 11:48 AM
dancgr edited the summary of this revision. (Show Details)Oct 17 2019, 1:04 PM
dancgr updated this revision to Diff 226731.Oct 28 2019, 12:54 PM

Updated diff to include missing aliases that are required

huntergr added inline comments.Oct 29 2019, 4:54 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
187

Nit: shouldn't be a space after MVT:: for the final valuetype.

llvm/lib/Target/AArch64/SVEInstrFormats.td
1809

For this encoding set we left the sdag pattern out of the class itself and used the helper SVE_3_Op_Pat instead.

e.g.

multiclass sve_int_bin_pred_log<bits<3> opc, string asm, SDPatternOperator op> {
  def _B : sve_int_bin_pred_arit_log<0b00, 0b11, opc, asm, ZPR8>;
  def _H : sve_int_bin_pred_arit_log<0b01, 0b11, opc, asm, ZPR16>;
  def _S : sve_int_bin_pred_arit_log<0b10, 0b11, opc, asm, ZPR32>;
  def _D : sve_int_bin_pred_arit_log<0b11, 0b11, opc, asm, ZPR64>;

  def : SVE_3_Op_Pat<nxv16i8, op, nxv16i1, nxv16i8, nxv16i8, !cast<Instruction>(NAME # _B)>;
  def : SVE_3_Op_Pat<nxv8i16, op, nxv8i1, nxv8i16, nxv8i16, !cast<Instruction>(NAME # _H)>;
  def : SVE_3_Op_Pat<nxv4i32, op, nxv4i1, nxv4i32, nxv4i32, !cast<Instruction>(NAME # _S)>;
  def : SVE_3_Op_Pat<nxv2i64, op, nxv2i1, nxv2i64, nxv2i64, !cast<Instruction>(NAME # _D)>;
}

In this case there's not much difference between approaches (both fulfil our goal of keeping basic patterns in SVEInstrFormats.td), but sometimes we need to ignore types and just match against register classes, or add in default predicates when matching unpredicated operations. Making more use of the helpers should make the code more maintainable, though our earlier codebase shared on github wasn't using them consistently.

I appreciate we haven't documented our intended use of helper functions or codegen conventions; I'll make a note to encourage more comments on them in future patches.

3085

I see that you have used the helper here (and thanks for adding the 2-op variant), but you could also use the helper for the nxv2i64 pattern as well instead of adding a set pattern to the encoding class.

3091

Not sure why these lines were changed.

dancgr updated this revision to Diff 226930.Oct 29 2019, 10:19 AM
dancgr marked 3 inline comments as done.

I have updated the code to reflect the comments

Hi @dancgr, thanks for this patch! Just to make sure we're not duplicating work here, @kmclaughlin is currently creating similar patches for all the FP intrinsics and masked gathers/scatters.

Hi @dancgr, thanks for this patch! Just to make sure we're not duplicating work here, @kmclaughlin is currently creating similar patches for all the FP intrinsics and masked gathers/scatters.

Hello! Sure, I currently am doing that for integer intrinsics, will keep out of those instructions.

dancgr updated this revision to Diff 226967.Oct 29 2019, 2:25 PM
dancgr edited the summary of this revision. (Show Details)
This comment was removed by dancgr.
dancgr updated this revision to Diff 226973.Oct 29 2019, 2:38 PM

Update pattern match to use the SVE_N_Op_Pat pattern where possible.

dancgr marked 2 inline comments as done.Oct 29 2019, 2:47 PM
dancgr added inline comments.
llvm/lib/Target/AArch64/SVEInstrFormats.td
1809

I've updated the code to follow this standard.

I just have one thing that I am not 100% sure about. So the helper patterns should be used always when possible. I don't know in which circumstance then I would use the pattern in the class over using a helper pattern. Should I avoid that whenever possible?

sdesmalen accepted this revision.Oct 30 2019, 3:08 AM

Thanks for the changes, LGTM.

This revision is now accepted and ready to land.Oct 30 2019, 3:08 AM
huntergr added inline comments.Oct 30 2019, 3:14 AM
llvm/lib/Target/AArch64/SVEInstrFormats.td
1809

Yes, I think it's preferable to use the helper where possible (at least for the SVE instruction formatters -- every backend seems to have their own conventions).