[PATCH] D97698: [RISCV] Support fixed-length INSERT_VECTOR_ELT

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck marked 2 inline comments as done.
frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2214
       return Op;
-    SDValue Mask, VL;
-    std::tie(Mask, VL) = getDefaultScalableVLOps(VecVT, DL, DAG, Subtarget);
-    SDValue Slidedown = DAG.getNode(RISCVISD::VSLIDEDOWN_VL, DL, VecVT,
-                                    DAG.getUNDEF(VecVT), Vec, Idx, Mask, VL);
+    SDValue Slidedown =
+        DAG.getNode(RISCVISD::VSLIDEDOWN_VL, DL, ContainerVT,
----------------
craig.topper wrote:
> frasercrmck wrote:
> > Am I right in thinking we don't need to slide down: we set the VL to `insert_index + 1` and slide a vector with the element in the first position up by `insert_index`, while tying the original vector to the output?
> I think that's right.
Cool, I'll tackle that in the near future.


================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll:23
+; RV32-NEXT:    vid.v v30
+; RV32-NEXT:    vsetvli a1, zero, e64,m2,ta,mu
+; RV32-NEXT:    vmseq.vi v0, v30, 3
----------------
craig.topper wrote:
> Was it intended to switch to VLMAX here?
No, that was an oversight. Should be fixed now. I'm just using the VL versions of SETCC and VSELECT for scalable vectors too, for consistency. We can drop back to the ISD versions if it proves a problem (optmization-wise).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97698