[PATCH] D97319: [RISCV] Support fixed vector extract element. Use VL=1 for scalable vector extract element.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck accepted this revision.
frasercrmck added a comment.
This revision is now accepted and ready to land.

Thanks, Craig. I'd taken a look at this lat week but the `i64` legalization issue was frustrating and I knew we had to improve the subvector support for it to work anyway.

LGTM other than the typo.



================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2866
+    // specific nodes if it needs legalization.
+    // FIXME: We would manually legalize if its important.
+    if (!isTypeLegal(Vec.getValueType()))
----------------
Minor typo: `it's`


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2874
     // Extract the lower XLEN bits of the correct vector element.
-    SDValue EltLo = DAG.getNode(RISCVISD::VMV_X_S, DL, XLenVT, Slidedown, Idx);
 
----------------
Huh, what was `Idx` doing there? That's odd.


================
Comment at: llvm/test/CodeGen/RISCV/rvv/extractelt-fp-rv32.ll:21
 ; CHECK-NEXT:    vslidedown.vi v25, v8, 2
+; CHECK-NEXT:    vsetvli zero, zero, e16,mf4,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v25
----------------
I suppose this in theory could also be done with a VL set to 1. I doubt it'd be any more performant, but it'd save a `vsetvli` in this situation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97319