[PATCH] D120894: [AArch64][SVE]Make better use of gather/scatter when inside a loop body

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12240
+static bool isInstUsedByGEP(Instruction *I) {
+  for (const Use &U : I->uses()) {
+    auto *UI = cast<Instruction>(U.getUser());
----------------
You may want to limit this recursion a bit further to some threshold, to avoid it becoming too expensive.

Maybe you can rewrite this to a loop, like this:

  unsigned Threshold = 3;  // I'd expect 3 or 4 to be sufficient for most cases.
  for (const User *U : I->users()) {
    for (unsigned J=0; J<Threshold; ++J) {
      if (isa<GetElementPtrInst>(U))
        return true;
      if (isa<PHINode>(U) || !U->hasOneUse())
        break;
      U = cast<Instruction>(U)->getUser();
    }
  }
  return false;


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12306
+    // and not check stepvector
+    IntrinsicInst *II = dyn_cast<IntrinsicInst>(I->getOperand(1));
+    if (II && II->getIntrinsicID() == Intrinsic::experimental_stepvector) {
----------------
This doesn't need to be limited to the second operand. You can just combine it with the case for splat.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12313-12315
+        auto Ext1 = cast<Instruction>(I->getOperand(i));
+        Ops.push_back(&Ext1->getOperandUse(0));
+        Ops.push_back(&I->getOperandUse(i));
----------------
I'm a bit confused about what this is doing, it seems like this is adding:

  I->getOperand(i)->getOperandUse(0);
  I->getOperandUse(i);

is that correct?

I notice it also hasn't checked the other operand yet before it returns `true`.

What I think you want to do is:
* First check if `I` is used by a GEP in some way.
* Then iterate through each of the operands, adding it to Ops if it's either a splat/stepvector.
* If any splats/stepvectors have been found, return.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120894