[PATCH] D69128: [AArch64][SVE] Add patterns for some integer vector instructions

[Graham Hunter via Phabricator via llvm-commits]

huntergr added inline comments.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:1809
 
-multiclass sve_int_bin_pred_log<bits<3> opc, string asm> {
-  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>;
+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, nxv16i8, nxv16i1, ZPR8, op>;
----------------
dancgr wrote:
> huntergr wrote:
> > 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.
> 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?
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).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69128/new/

https://reviews.llvm.org/D69128