[PATCH] D97475: [RISCV] Support EXTRACT_SUBVECTOR on vector masks

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2535
+      OrigIdx /= 8;
+      SubVecVT = SubVecVT.MVT::getVectorVT(
+          MVT::i8, SubVecVT.getVectorMinNumElements() / 8,
----------------
What does "SubVecVT.MVT::getVectorVT" mean?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2551
+      MVT ExtSubVecVT = SubVecVT.changeVectorElementType(MVT::i8);
+      Vec = DAG.getZExtOrTrunc(Vec, DL, ExtVecVT);
+      Vec = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, ExtSubVecVT, Vec,
----------------
I know it takes more to write, but can we just use ISD::ZERO_EXTEND here and ISD::TRUNCATE after. Maybe we should have getZExt or getTrunc helper? I've noticed a preference for using getZExtOrTrunc in reviews over the years even in cases when the direction is known. But I feel like it makes the code more confusing for future readers.


================
Comment at: llvm/test/CodeGen/RISCV/rvv/extract-subvector.ll:399
+; CHECK-NEXT:    vsetvli a0, zero, e8,mf4,ta,mu
+; CHECK-NEXT:    vand.vi v25, v25, 1
+; CHECK-NEXT:    vmsne.vi v0, v25, 0
----------------
This vand.vi isn't necessary. But I'm not sure the best way to remove it in the general case of truncate. For this specific transform we could just emit the compare directly instead of going through truncate?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97475