[PATCH] D69392: [ARM] MVE interleaving load and stores.

[Dave Green via Phabricator via llvm-commits]

dmgreen marked 3 inline comments as done.
dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:16718
   // the vector types are divisible by 128.
-  if (!Subtarget->hasNEON() || !isLegalInterleavedAccessType(VecTy, DL))
+  if (!isLegalInterleavedAccessType(Factor, VecTy, DL))
     return false;
----------------
samparker wrote:
> What is Factor here? Is it not the number of elements..?
Factor is the "n" in "vldn". So 2 or 4 for MVE, but also 3 for Neon. It comes from the elements of the shuffle, like the example in the comment at the start of this function.

We really just need it to say that a VLD3 isn't legal on MVE.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:16774
+      Ops.push_back(Builder.CreateBitCast(BaseAddr, VecEltTy));
+      dbgs() << *BaseAddr << "\n";
+      return Builder.CreateCall(VldnFunc, Ops, "vldN");
----------------
samparker wrote:
> debug
Ah!


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:16943
+        Ops.push_back(S);
+      Ops.push_back(Builder.getInt32(0));
+      Builder.CreateCall(VstNFunc, Ops);
----------------
samparker wrote:
> How about performing this with a loop?
Yeah. Nice suggestion. It involves a pop because of the last item changing, but looks better.


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

https://reviews.llvm.org/D69392