[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/lib/Target/ARM/ARMISelDAGToDAG.cpp:1610
+  unsigned Align = LD->getAlignment();
+  bool IsLE = Subtarget->isLittle();
+
----------------
bail here for BE?


================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:1612
+
+  if (Align >= 2 && LoadedVT == MVT::v4i16 &&
+      SelectT2AddrModeImm7Offset(N, LD->getOffset(), Offset, 1)) {
----------------
Why the >= comparisons for the alignment? Is that right?


================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:1631
+  } else if (Align >= 4 &&
+             (IsLE || LoadedVT == MVT::v4i32 || LoadedVT == MVT::v4f32) &&
+             SelectT2AddrModeImm7Offset(N, LD->getOffset(), Offset, 2))
----------------
I am wondering if this is logically correct, the `(IsLE || ...` in particular. 
Perhaps we don't need the IsLE check here, if we bail for BE earlier.


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

https://reviews.llvm.org/D63840