[PATCH] D115546: [RISCV][VP] Add RVV codegen for [nX]vXi1 vp.select

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added a reviewer: RKSimon.
frasercrmck added a subscriber: RKSimon.
frasercrmck added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1241
+SDValue VectorLegalizer::ExpandVP_SELECT(SDNode *Node) {
+  // Implent VP_SELECT in terms of VP_XOR, VP_AND and VP_OR on platforms which
+  // do not support it natively.
----------------
nit: `Implement`


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1248
+  SDValue Op2 = Node->getOperand(2);
+  SDValue Length = Node->getOperand(3);
+
----------------
nit: `EVL` would be a more appropriate name, I think.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1253
+  // If we can't even use the basic vector operations of
+  // VP_AND,VP_OR,XOR, we will have to scalarize the op.
+  if (TLI.getOperationAction(ISD::VP_AND, VT) == TargetLowering::Expand ||
----------------
nit: `VP_XOR`


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1258
+    return DAG.UnrollVectorOp(Node);
+
+  auto NotMask = [&]() {
----------------
The following expansion is only correct under certain conditions: when the vselect condition and operand types are identically-sized integer vectors and when the mask is -1/0. See the original `ExpandVSELECT` for that.

Since we don't have an in-tree test case for anything other than expanding [nx]vXi1 VSELECT maybe it's sufficient for now to resort to `UnrollVectorOp` whenever the operand types aren't i1 vectors? `UnrollVectorOp` will no doubt fail for `VP_SELECT` right now, but we wouldn't be making anything worse.

Not sure: any thoughts @craig.topper? @RKSimon?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115546