[PATCH] D67828: [ARM] Split large truncating MVE stores

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:13169
   EVT VT = StVal.getValueType();
-  if (St->isTruncatingStore() && VT.isVector()) {
+  if (Subtarget->hasNEON() && St->isTruncatingStore() && VT.isVector()) {
     SelectionDAG &DAG = DCI.DAG;
----------------
To make this function easier to read, how about extracting the truncating neon and non-truncating mve stuff out into their own functions?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:13250
+      return SDValue();
+    SDValue Ext = St->getValue();
+    if (Ext->getOpcode() != ISD::TRUNCATE)
----------------
Nit: I'm guessing this is taken from some load code, so I think Ext should be renamed to something a bit more descriptive.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:13261
+
+    unsigned Multiple = 0;
+    if (FromEltVT == MVT::i32 && (ToEltVT == MVT::i16 || ToEltVT == MVT::i8))
----------------
Nit: NumElements..?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:13271
+    SDLoc DL(St);
+    // Details about the old load
+    SDValue Ch = St->getChain();
----------------
old load?!


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

https://reviews.llvm.org/D67828