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

[David Sherwood via Phabricator via llvm-commits]

david-arm added a comment.

Hi @CarolineConcatto, I just have a few more minor comments!



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12240
+static bool isInstUsedByGEP(Instruction *I, unsigned Threshold) {
+  // Threshold avoids recursivity becoming too expensive
+  if (Threshold == 0)
----------------
nit: Just a minor comment, but perhaps it's simpler to explain this as something like:

  // Use a threshold to limit the amount of recursion.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12245
+    auto *UI = cast<Instruction>(U.getUser());
+    if (isa<PHINode>(UI))
+      return false;
----------------
nit: Perhaps these can be combined into a single statement, i.e.

  if (isa<PHINode>(UI)) || !UI->hasOneUse())
    return false;

to reduce lines of code?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12249
+      return false;
+    if (dyn_cast<GetElementPtrInst>(UI))
+      return true;
----------------
I think it's more common to use

  if (isa<GetElementPtrInst>(UI))

here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120894