[PATCH] D95804: [ARM] MVE vector lane interleaving

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVELaneInterleavingPass.cpp:1
+//===- MVELaneInterleaving.cpp - Inverleave for MVE instructions ----------===//
+//
----------------
Before I do a 2nd and more in depth review, some name bike shedding: Lane Interleaving.
I am wondering if that is the most accurate description. This pass is about more efficient vector (element) extensions, which is achieved by some reshuffling. But lane interleaving is something that is used under the hood so we can use the bottom/top moves, thus it's not about lane interleaving per se or more efficient codegen for lane interleaving.  So, would MVEVectorExtends or something like that be a better description?


================
Comment at: llvm/lib/Target/ARM/MVELaneInterleavingPass.cpp:80
+    "enable-mve-interleave", cl::Hidden, cl::init(true),
+    cl::desc("Enable interleave mve vector operation lowering"));
+
----------------
Nit: perhaps mve should be MVE here.


================
Comment at: llvm/lib/Target/ARM/MVELaneInterleavingPass.cpp:114
+
+static bool looksBeneficial(SmallSetVector<Instruction *, 4> &Exts,
+                            SmallSetVector<Instruction *, 4> &Truncs) {
----------------
Bike shedding names: `looksBeneficial` sound a bit uncertain. Perhaps just IsBeneficial or something along those lines.


================
Comment at: llvm/lib/Target/ARM/MVELaneInterleavingPass.cpp:158
+
+  // Look for connected operantions starting from Ext's, terminating at Truncs.
+  std::vector<Instruction *> Worklist;
----------------
Typo: operantions


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

https://reviews.llvm.org/D95804