[PATCH] D71215: [AArch64][SVE] Add patterns for unpredicated load/store to frame-indices.

[Eli Friedman via Phabricator via llvm-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM with one minor suggestion.

Sorry about the delay; I didn't understand the dependency relationship between this and D72758 <https://reviews.llvm.org/D72758>.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15735
+  // With scalable vectors it is not known how many consecutive stores could
+  // make up a wider one due to vscale being unknown at compile-time, so bail out.
   EVT MemVT = St->getMemoryVT();
----------------
This comment doesn't seem quite right.  We could theoretically merge two stores if they're both scalable. For example, two `<vscale x 8 x i8>` stores can be merged to one `<vscale x 16 x i8>` store; we know `<vscale x 16 x i8>` is exactly twice as large as `<vscale x 8 x i8>`.

You'd need extra logic for that, though, so I'm not suggesting changing the code.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:1267
+  defm Pat_Load_P4  : unpred_load_predicate<nxv4i1, LDR_PXI>;
+  defm Pat_Load_P2  : unpred_load_predicate<nxv2i1, LDR_PXI>;
 }
----------------
nxv2i1 has the same memory layout as nxv16i1?  I guess that makes sense given the available instructions.  We might need to modify the datalayout to make that work properly; I think, without any explicit guidance from the layout string, it will assume a nxv2i1 load reads "vscale" bytes, not "vscale * 2" bytes.

Not something to change in this patch, of course.


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

https://reviews.llvm.org/D71215