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

[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:174-188
+    Value *GEPPtr = GEP->getPointerOperand();
+    if (GEPPtr->getType()->isVectorTy()) {
+      // FIXME: In this case, we could try to build a QI gather load
+      LLVM_DEBUG(dbgs() << "masked gathers: gather from a vector of pointers"
+                        << " hidden behind a getelementptr currently not"
+                        << " supported. Expanding.\n");
+      return false;
----------------
dmgreen wrote:
> These return false's are worrying me a little. There may be cases where we cannot select this as a GEP, but it would be fine to fall back to selecting the qi variant instead. Maybe something like this, for example:
> ```
> define arm_aapcs_vfpcc <4 x i32> @qi4(<4 x i32*> %p) {                                                                   
> entry:                                                                                                                   
>   %g = getelementptr inbounds i32, <4 x i32*> %p, i32 4                                                                  
>   %gather = call <4 x i32> @llvm.masked.gather.v4i32.v4p0i32(<4 x i32*> %g, i32 4, <4 x i1> <i1 true, i1 true, i1 true, i1 true>, <4 x i32> undef)                                                                                                
>   ret <4 x i32> %gather                                                                                                  
> }                                                                                                                        
> ```
> It can also be selected as a `VLDRW.u32 Qd, [P, 4]` I think, but we should be able to get a `VLDRW.u32 Qd, [g]` at least. I believe you mentioned something similar to this when talking about scatter lowering.
> 
> Also it probably makes sense to check these basic things about the GEP before the offset code above. (Although I think any GEP we see will always has at least 1 offset, it makes conceptual sense to check the number of operands before using them).
You are right in this case, and maybe there are others we did not think of. I've modularised the code now which makes it easier to fall back to QI if the construction of a more complex gathers fails for some reason. Hopefully this also makes the code more readable.


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

https://reviews.llvm.org/D72330