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

[Joe Ellis via Phabricator via llvm-commits]

joechrisellis added inline comments.


================
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);
----------------
kmclaughlin wrote:
> Can we remove some of the indentation below by returning SDValue() here if the offset isn't extended instead?
I would prefer to leave this as-is, because this was an intentional decision. It's possible that we might want to extend this function with a new optimisation pattern. It'd be easier to extend the function in its current state than it would be with the early return. 🙂 


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14416
+                              : AArch64ISD::GLD1S_SXTW_SCALED_MERGE_ZERO;
+        break;
+      default:
----------------
kmclaughlin wrote:
> 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);
> ```
Thanks, that's cleaned things up a lot. 🙂 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97858