[PATCH] D63840: [ARM] Add support for MVE pre and post inc loads and stores.

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/include/llvm/Target/TargetSelectionDAG.td:1114
+def pre_truncstvi16 : PatFrag<(ops node:$val, node:$base, node:$offset),
+                             (pre_truncst node:$val, node:$base, node:$offset)> {
+  let IsStore = 1;
----------------
nit: indent off by 1


================
Comment at: llvm/include/llvm/Target/TargetSelectionDAG.td:1156
+def post_truncstvi8 : PatFrag<(ops node:$val, node:$base, node:$offset),
+                             (post_truncst node:$val, node:$base, node:$offset)> {
+  let IsStore = 1;
----------------
here


================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:151
+  bool SelectT2AddrModeImm7Offset(SDNode *Op, SDValue N,
+                                 SDValue &OffImm);
+  bool SelectT2AddrModeImm7Offset(SDNode *Op, SDValue N,
----------------
and here


================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:153
+  bool SelectT2AddrModeImm7Offset(SDNode *Op, SDValue N,
+                                 SDValue &OffImm, unsigned Shift);
+  template<unsigned Shift>
----------------
and here


that's it, just nitpicking! :-)


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:332
+  // Pre and Post inc on these are legal, given the correct extends
+  for (unsigned im = (unsigned)ISD::PRE_INC;
+       im != (unsigned)ISD::LAST_INDEXED_MODE; ++im) {
----------------
perhaps we could have a little bit of fun with c++ and do something like this:

  for (unsigned I : ISD::pre_inc_ins())

where pre_inc_ins() creates an iterator range, similar to what happens in MachineValueType.h


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

https://reviews.llvm.org/D63840