[PATCH] D112076: [AArch64][SVE][InstCombine] Combine contiguous gather/scatter to load/store

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:880
+  Value *IndexBase;
+  if (match(Index, m_Intrinsic<Intrinsic::aarch64_sve_index>(
+                       m_Value(IndexBase), m_SpecificInt(1)))) {
----------------
nit: Is it worth reversing the logic to remove indentation and avoid unnecessary creation of variables above? For example, something like this:

  Value *Index = II.getOperand(2);
  Value *IndexBase;
  if (!match(Index, m_Intrinsic<Intrinsic::aarch64_sve_index>(
      m_Value(IndexBase), m_SpecificInt(1))))
    return None;

  Value *Mask = II.getOperand(0);
  Value *BasePtr = II.getOperand(1);
  ...




================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:897
+    MaskedLoad->takeName(&II);
+    return IC.replaceInstUsesWith(II, MaskedLoad);
+  }
----------------
Should we also call `II.eraseFromParent();` here too like the scatter case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112076