[PATCH] D118345: [AArch64][SVE] Add more folds to make use of gather/scatter with 32-bit indices

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16182
+      Index.getOperand(0).getOperand(0).getOpcode() == ISD::STEP_VECTOR) {
+    if (auto Shift = DAG.getSplatValue(Index.getOperand(1)))
+      if (auto Offset = DAG.getSplatValue(Index.getOperand(0).getOperand(1))) {
----------------
Does the following work:

  if (auto *Shift = dyn_cast_or_null<ConstantSDNode>(DAG.getSplatValue(Index.getOperand(1))) {
    ..
  }

?

If so, that removes the need to do `auto *C = dyn_cast<ConstantSDNode>(Shift)` and the return if C == nullptr.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16190
+                           ->getSExtValue();
+        // Stride = const << shift
+        Stride = Step << C->getSExtValue();
----------------
nit: the comment as-is seems redundant.

I think it's worth saying though that Stride is not scaled explicitly by 'Scale', because this is done implicitly by the gather/scatter addressing mode.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16192-16193
+        Stride = Step << C->getSExtValue();
+        Offset = DAG.getNode(ISD::MUL, DL, MVT::i64, Offset, N->getScale());
+        // BasePtr = BasePtr + (Offset << Shift)
+        Offset = DAG.getNode(ISD::SHL, DL, MVT::i64, Offset, Shift);
----------------
nit: Change to:

  // BasePtr = BasePtr + ((Offset * Scale) << Shift)
  Offset = DAG.getNode(ISD::MUL, DL, MVT::i64, Offset, N->getScale());


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118345