[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:131
 
-  if (Ty->getVectorNumElements() != 4)
-    // Can't build an intrinsic for this
-    return false;
-  if (match(Mask, m_One()))
-    Load = Builder.CreateIntrinsic(Intrinsic::arm_mve_vldr_gather_base,
-                                   {Ty, Ptr->getType()},
-                                   {Ptr, Builder.getInt32(0)});
-  else
-    Load = Builder.CreateIntrinsic(
-        Intrinsic::arm_mve_vldr_gather_base_predicated,
-        {Ty, Ptr->getType(), Mask->getType()},
-        {Ptr, Builder.getInt32(0), Mask});
+  unsigned Unsigned = 1;
+  Value *BasePtr = Ptr;
----------------
samparker wrote:
> Doesn't look this needs to be a variable?
No, it doesn't.


================
Comment at: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp:170
+      }
+      if (isa<VectorType>(C->getType()))
+        C = C->getSplatValue();
----------------
samparker wrote:
> I think you can just call C->isNullValue().
I didn't realise that function exists, thanks for the hint


================
Comment at: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp:178
+
+    Type *OffsType = VectorType::getInteger(cast<VectorType>(Ty));
+    // If the offset we found does not have the type the intrinsic expects,
----------------
samparker wrote:
> How about moving this logic up to around line 155 so that all the offset stuff is together, it's quite hard to follow scrolling up and down!
That makes sense, I'll move it.


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

https://reviews.llvm.org/D72330