[PATCH] D76786: [ARM][MVE] Add support for incrementing gathers

[Anna Welker via Phabricator via llvm-commits]

anwel marked 2 inline comments as done.
anwel added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp:495
+    return cast<IntrinsicInst>(
+        tryCreateMaskedGatherBase(I, OffsetsIncoming, Builder, Immediate));
+  else
----------------
dmgreen wrote:
> I feel like this should be using BasePtr somewhere?
You are right, the base pointer should be added to the offsets before handing them over to the intrinsic construction. I fixed that now.


================
Comment at: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp:502
+
+Value *MVEGatherScatterLowering::tryCreateIncrementingWBGather(
+    IntrinsicInst *I, Value *BasePtr, Value *Offsets, unsigned TypeScale,
----------------
dmgreen wrote:
> This looks OK. Can you add one extra check somewhere that gep has a single user? As the phi is changing and the gep uses the phi.
You are right, unfortunately we cannot allow this as is with gep having more than one user. This means for now that in cases where multiple gathers and/or scatters use the same gep we cannot make any of them write the increment back, which we might want to fix in a future patch.


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

https://reviews.llvm.org/D76786