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

[Fraser Cormack via Phabricator via llvm-commits]

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,
----------------
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?


================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll:55
 ; RV32:       # %bb.0:
+; RV32-NEXT:    addi sp, sp, -16
+; RV32-NEXT:    .cfi_def_cfa_offset 16
----------------
The codegen here isn't ideal, but legalization is kicking in for the `i64` value and it's following the "normal" path. Then there comes a build_vector which is expanded through the stack. Should we do something else? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97698