[PATCH] D119834: [RISCV] Add fixed-length vector instrinsics for segment load

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4838
+    unsigned NF = Op->getNumValues() - 1;
+    assert(NF >= 2 && NF <= 8 && "Unexcepted seg number");
+    MVT XLenVT = Subtarget.getXLenVT();
----------------
Unexcepted -> Unexpected


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4846
+    auto *Load = cast<MemIntrinsicSDNode>(Op);
+    SmallVector<SDValue, 8> Ops{Load->getChain(), IntID};
+    Ops.push_back(Op.getOperand(2)); // Ptr
----------------
You don't need a SmallVector, the number of items is fixed. You can use a plain array.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4850
+
+    SmallVector<EVT, 8> ContainerVTs;
+    for (unsigned int RetIdx = 0; RetIdx < NF; RetIdx++) {
----------------
I think you can do 

```
SmallVector<EVT, 8> ContainerVTs(NF, ContainerVT);
```

and avoid the loop.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4860
+    SmallVector<SDValue, 8> Results;
+    for (unsigned int RetIdx = 0; RetIdx < NF; RetIdx++) {
+      Results.push_back(convertFromScalableVector(VT, Result.getValue(RetIdx),
----------------
Drop the curly braces around a single line body.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119834