[PATCH] D115259: [AArch64][SVE] Lower vector.insert to predicated SEL

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10999-11010
+  if (Idx == 0 && isPackedVectorType(VT, DAG) && !Op.getOperand(0).isUndef()) {
+    unsigned int PredPattern =
+        getSVEPredPatternFromNumElements(InVT.getVectorNumElements());
+    auto PredTy = VT.changeVectorElementType(MVT::i1);
+    SDValue PTrue = getPTrue(DAG, DL, PredTy, PredPattern);
+    SDValue ScalableVec1 = convertToScalableVector(DAG, VT, Vec1);
+    return DAG.getNode(ISD::VSELECT, DL, VT, PTrue, Vec0, ScalableVec1);
----------------
paulwalker-arm wrote:
> Perhaps worth combining these blocks? Like
> 
> ```
> if (Idx == 0 && isPackedVectorType(VT, DAG)) {
>   // This will be matched by custom code during ISelDAGToDAG.
>   if (Vec0.isUndef())
>     return Op;
> 
>   unsigned int PredPattern = ......
>   ......
> }
> ```
The two blocks differ slightly:
`isPackedVectorType(VT, DAG)`
and
`isPackedVectorType(InVT, DAG)`

I had to pass VT instead of InVT to get isPackedVector to return false on the <4 x bfloat> test. All tests pass when I combine the statements and use VT. Do you think there will be any side effects later down the line from doing this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115259