[PATCH] D99355: Implementation of intrinsic and SDNode definitions for VP load, store, gather, scatter.

[Vineet Kumar via Phabricator via llvm-commits]

vkmr added a comment.

Thanks Hussain for this patch!
 For now I have a couple of minor comments, I will take a deeper look later.
Also, I don't quite understand yet how VP handles (or plans to handle) specifying alignment parameter. Does it need VPBuilder (not to be confused with VPlan VPBuilder!) for that? Would it make sense to add the Alignment parameter to VP load/store/scatter/gather intrinsics (much like their masked counterparts) for now, unless there is already a way to specify that?



================
Comment at: llvm/include/llvm/IR/Intrinsics.td:1341
+// Memory Intrinsics
+def int_vp_store : Intrinsic<[],
+                             [ llvm_anyvector_ty,
----------------
Better to use `DefaultAttrsIntrinsic`? (Ditto for other load, gather and scatter)


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:1366
+                              [ IntrArgMemOnly, IntrNoSync, IntrWillReturn ]>;
+// TODO allow IntrNoCapture for vectors of pointers
 
----------------
minor style nit: A blank line above TODO. Perhaps reposition it to above the scope where it applies.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99355