[PATCH] D105871: Followup to D99355: implementation of sdag support for vp load/store/gather/scatter.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/SelectionDAGNodes.h:2347
+
+  // VPLoadSDNode (Chain, ptr, offset, mask, VLen)
+  // VPStoreSDNode (Chain, data, ptr, offset, mask, VLen)
----------------
nit: inconsistent capitalization, plus `VLen` is maybe better known at `EVL`.

Also it doesn't have to be `i32`. It may be widened to a target-specific value, e.g.,
```
  MVT EVLParamVT = TLI.getVPExplicitVectorLengthTy();
```
Not sure how best to describe that in the comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7596
+    ISD::MemIndexedMode AM, ISD::LoadExtType ExtType, EVT VT, const SDLoc &dl,
+    SDValue Chain, SDValue Ptr, SDValue Offset, SDValue Mask, SDValue VLen,
+    MachinePointerInfo PtrInfo, EVT MemVT, Align Alignment,
----------------
I think all of these `VLen` params may be better as `EVL` since that's how they're known in the rest of the code. Any thoughts, @simoll?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105871