[PATCH] D72330: [ARM][MVE] Enable masked gathers from base + vector of offsets

[Anna Welker via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp:176
+    Value *GEPPtr = GEP->getPointerOperand();
+    if (!GEPPtr->getType()->isVectorTy())
+      BasePtr = GEPPtr;
----------------
dmgreen wrote:
> Do we have and tests where this is false? Is it for something like `getelementptr <4 x i32>, <4 x i32> *%base, i64 16`? That might make sense to generate a QI gather load from (although I don't know if anything will generate gep's like that at the moment without some help).
> 
> For the RQ form, we need to make sure that base is a single pointer.
Yes, I will change this to a check of whether we do have a vector of pointers and, if that rare case occurs, leave it to expansion for now. I'll add a test for that behaviour.


================
Comment at: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp:189
+    LLVMContext &C = I->getContext();
+    if (BasePtr->getType() == Type::getInt32PtrTy(C)
+        && Ty->getScalarSizeInBits() == 32) {
----------------
dmgreen wrote:
> I think this can be something like GEP->getResultElementType()->getPrimitiveSizeInBits() == 32. Same below, so maybe make a variable for that size. (scaled_v8f16_half should be be matched too, I think).
> 
> Also Clang format!
You are right, that is also getting rid of the context variable. I will refactor the code a bit.


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

https://reviews.llvm.org/D72330