[PATCH] D111221: [AArch64][SVE] Improve code generation for VLS i1 masks

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15737
+static SDValue performSunpkloCombine(SDNode *N, SelectionDAG &DAG) {
+  // sunpklo (mov z, p/z, -1) => mov z, (punpklo p), -1
+  if (N->getOperand(0).getOpcode() == ISD::SIGN_EXTEND &&
----------------
This comment doesn't match the code below.  At the DAG level this is really `sunpklo(sext(pred)) -> sext(extract_low_half(pred))`.

Also, is this transform universally good?  As in, if the later transformation does not occur, is punpklo always better than sunpklo?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15745
+                               DAG.getVectorIdxConstant(0, SDLoc(N)));
+    SDValue Sext = DAG.getSExtOrTrunc(Unpk, SDLoc(N), N->getValueType(0));
+    return Sext;
----------------
Can you be explicit here given we know we always want a sign extend?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16438
+      LHS->getOperand(0)->getConstantOperandVal(1) == 0) {
+    auto OrigPred = LHS->getOperand(0)->getOperand(0)->getOperand(0);
+    if (Pred.getOpcode() == AArch64ISD::PTRUE &&
----------------
Perhaps I've confused myself with the number of `getOperand()` calls but this doesn't looks quite right:
```
LHS == ISD::SIGN_EXTEND
LHS->getOperand(0) == ISD::EXTRACT_SUBVECTOR
LHS->getOperand(0)->getOperand(0) == vector we're extracting the subvector from
LHS->getOperand(0)->getOperand(0)->getOperand(0) == ???? and how do you know LHS->getOperand(0)->getOperand(0) has an operand to get?
```

I can see the following code then ensures `LHS->getOperand(0)->getOperand(0)->getOperand(0)` is a `PTRUE` but before then depending on `LHS->getOperand(0)->getOperand(0)` the extra call to `getOperand(0)` might assert.  Also, is it really the case that is doesn't matter how the `PTRUE` is processed before being passed to `ISD::EXTRACT_SUBVECTOR`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111221