[PATCH] D105090: [ARM] Introduce MVEEXT ISel lowering

[Sam Tebbs via Phabricator via llvm-commits]

samtebbs added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:9007
+  EVT ExtVT = ToVT.getHalfNumVectorElementsVT(*DAG.getContext());
+  if (ToVT.getScalarType() == MVT::i32 && FromVT.getScalarType() == MVT::i8)
+    ExtVT = MVT::v8i16;
----------------
I think it would be useful to add a comment here explaining the intermediate extension.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:17375
 
+static SDValue PerformSplittingMVEEXTToWideningLoad(SDNode *N, SelectionDAG &DAG) {
+  SDValue N0 = N->getOperand(0);
----------------
I think this could do with a comment at the beginning detailing what it's doing.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:17377-17380
+  if (N0.getOpcode() != ISD::LOAD)
+    return SDValue();
+  LoadSDNode *LD = cast<LoadSDNode>(N0.getNode());
+  if (!LD->isSimple() || !N0.hasOneUse() || LD->isIndexed())
----------------
Is it possible to merge the opcode check and the cast to a dyn_cast and checking `LD` in the second if statement?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:17440
+
+SDValue ARMTargetLowering::PerformMVEExtCombine(
+    SDNode *N, TargetLowering::DAGCombinerInfo &DCI) const {
----------------
This function could also do with an explanatory comment.


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

https://reviews.llvm.org/D105090