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

[Dave Green via Phabricator via llvm-commits]

dmgreen 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;
----------------
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).


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

https://reviews.llvm.org/D72330