[PATCH] D116407: [LegalizeTypes][VP] Add widening support for vp.select

Mon Jan 3 03:46:53 PST 2022

frasercrmck added a comment.

I think the legalization itself is sound: just nits.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:4530

-SDValue DAGTypeLegalizer::WidenVecRes_SELECT(SDNode *N) {
+SDValue DAGTypeLegalizer::WidenVecRes_SELECT(SDNode *N, bool IsVP) {
   EVT WidenVT = TLI.getTypeToTransformTo(*DAG.getContext(), N->getValueType(0));
----------------
Similar comment to D116400 really - rename this function? Though arguably a precedent has already been set here since `SELECT` and `VSELECT` were both already using this function. So I'm not sure.

================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-vselect-vp.ll:7

+declare <5 x i8> @llvm.vp.select.v5i8(<5 x i1>, <5 x i8>, <5 x i8>, i32)
+
----------------
Maybe move this in between `<4 x i8>` and `<8 x i8>`? That way there's a progression of vector widths.

================
Comment at: llvm/test/CodeGen/RISCV/rvv/vselect-vp.ll:7

+declare <vscale x 14 x i8> @llvm.vp.select.nxv14i8(<vscale x 14 x i1>, <vscale x 14 x i8>, <vscale x 14 x i8>, i32)
+
----------------
Maybe move this in between `<vscale x 8 x i8>` and `<vscale x 16 x i8>`?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116407