[PATCH] D97858: [AArch64][SVE] Fold vector ZExt/SExt into gather loads where possible

[Kerry McLaughlin via Phabricator via llvm-commits]

kmclaughlin added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14373-14377
+  SDValue Op0 = N->getOperand(0); // Ch
+  SDValue Op1 = N->getOperand(1); // Pg
+  SDValue Op2 = N->getOperand(2); // Base
+  SDValue Op3 = N->getOperand(3); // Offset
+  SDValue Op4 = N->getOperand(4); // Ty
----------------
nit: I think it might make the code below a bit clearer if these variables were named similarly to the comments accompanying them, i.e. Chain, Pred, Base, etc


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14388
+  // Fold sign/zero extensions of vector offsets into GLD1 nodes where possible.
+  if (OffsetIsSExt || OffsetIsZExt) {
+    SDValue ExtPg = Op3.getOperand(0);
----------------
Can we remove some of the indentation below by returning SDValue() here if the offset isn't extended instead?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14416
+                              : AArch64ISD::GLD1S_SXTW_SCALED_MERGE_ZERO;
+        break;
+      default:
----------------
There were some helper functions added for LowerMGATHER which I think you might be able to use here to get the right gather opcode (getGatherVecOpcode & getSignExtendedGatherOpcode).
As an example:

```
...
case GLD1_MERGE_ZERO:
case GLD1S_MERGE_ZERO:
  getGatherVecOpcode(false /*Scaled*/, OffsetIsSext, true /*NeedsExtend*/);
case GLD1_SCALED_MERGE_ZERO:
case GLD1_SCALED_MERGE_ZERO:
  getGatherVecOpcode(true /*Scaled*/, OffsetIsSext, true /*NeedsExtend*/);
...
if (Opc == GLD1S_MERGE_ZERO || Opc == GLD1S_SCALED_MERGE_ZERO)
  NewOpc = getSignExtendedGatherOpcode(NewOp);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97858