Implementation of intrinsic and SDNode definitions for VP load, store, gather, scatter.

[hussainjk]

Summary

This patch adds intrinsic definitions and SDNodes for predicated load/store/gather/scatter, based on the work done in D57504.

[vkmr]

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?

llvm/include/llvm/IR/Intrinsics.td

1372

Better to use DefaultAttrsIntrinsic? (Ditto for other load, gather and scatter)

1397

minor style nit: A blank line above TODO. Perhaps reposition it to above the scope where it applies.

[simoll]

In D99355#2653260, @vkmr wrote:

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?

We should use the align(N) attribute on pointers to convey the alignment. That's what we do in the LLVM for SX-Aurora stack (https://github.com/sx-aurora-dev/llvm-project/blob/683f41073689262a8a493cce7caba13bbccea56c/llvm/test/CodeGen/VE/Packed/vp_load_packed.ll#L80) . I'd say that the alignment parameter in the pre-existing masked.load/store simply predates that attribute.
And yes, setting alignment is something that the VPBuilder or what's-it-called-after-bikeshedding would take care of for you.

[simoll]

llvm/unittests/IR/VPIntrinsicTest.cpp

34–54

There are no mask or vlen attributes in upstream.

[simoll]

Can we split this up into a patch with only the intrinsics and a second patch with SelectionDAG?

[hussainjk]

Reverted some SDNode bits.

[hussainjk]

Various code review fixes.

[craig.topper]

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

1369–1370

Can we override clang-format here? The previous code had a more consistent structure.

llvm/include/llvm/IR/IntrinsicInst.h

421

It kind of feels like maybe there should be a VPMemIntrinsic and a VPStoreIntrinsic subclasses that contain these methods? Then you can use the more common isa, cast, dyn_cast and not have Optional return values.

llvm/lib/IR/IntrinsicInst.cpp

347

Doesn't the getValue() below already assert if there is no value?

[frasercrmck]

Just some minor comments. I think Craig's suggestion of mem subclasses sounds like it's worth investigating.

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

1369–1370

Alternatively, I suppose, it could be a switch statement (aside from the last two)? Presumably that'd be optimized well enough by compilers?

llvm/include/llvm/IR/IntrinsicInst.h

428

Inconsistent documentation style, e.g. return the and return The, and use of commas vs forward slashes.

Also, is clang-tidy correct here? No uppercase letter on static methods?

llvm/include/llvm/IR/Intrinsics.td

1377

How some the loads are IntrReadMem but the stores aren't IntrWriteMem?

llvm/include/llvm/IR/VPIntrinsics.def

92

typo: cannonical -> canonical

llvm/lib/IR/IntrinsicInst.cpp

353

Purely a matter of style but this is slightly shorter and (I think) a little clearer:

if (auto PtrParamOpt = GetMemoryPointerParamPos(getIntrinsicID()))
  return getArgOperand(PtrParamOpt.getValue());
return nullptr;

[hussainjk]

Addressing review comments.

[hussainjk]

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

1369–1370

It seems like this style is more idiomatic in the surrounding code, so leaving it as-is for now (with spacing fixed).

llvm/include/llvm/IR/IntrinsicInst.h

421

There will be separate classes for the corresponding SDNodes at least, in the followup patch. A bunch of the code there also assumes you can generically call these methods, so I think it would be good to leave it like this for now and revisit it in the followup patch to minimize things breaking.

[craig.topper]

llvm/include/llvm/IR/IntrinsicInst.h

428

Should this be getMemoryPointerParamPos as clang-format says?

432

Same here

[hussainjk]

Addressing review comments.

[frasercrmck]

I had a thought about alignment. Do we have a plan on how to deal with unaligned accesses? The other non-VP masked intrinsics are scalarized by the ScalarizeMaskedMemIntrin pass and regular loads and stores are handled by -- I believe -- LegalizeDAG.

This doesn't apply for scalable vectors, where presumably the onus is on the author/compiler transform not to emit them in the first place. But then for that we have TTI methods like isLegalMaskedStore and friends.

[simoll]

In D99355#2781282, @frasercrmck wrote:

I had a thought about alignment. Do we have a plan on how to deal with unaligned accesses?

Not yet

The other non-VP masked intrinsics are scalarized by the ScalarizeMaskedMemIntrin pass and regular loads and stores are handled by -- I believe -- LegalizeDAG.

Maybe vp can piggyback on ScalarizeMaskedMemIntrin for unaligned access expansion.

This doesn't apply for scalable vectors, where presumably the onus is on the author/compiler transform not to emit them in the first place. But then for that we have TTI methods like isLegalMaskedStore and friends.

ScalarizeMaskedMemIntrin could use llvm.experimental.vector.extract to do the same for scalable vectors...

[hussainjk]

In D99355#2784442, @simoll wrote:

In D99355#2781282, @frasercrmck wrote:

I had a thought about alignment. Do we have a plan on how to deal with unaligned accesses?

Not yet

The other non-VP masked intrinsics are scalarized by the ScalarizeMaskedMemIntrin pass and regular loads and stores are handled by -- I believe -- LegalizeDAG.

Maybe vp can piggyback on ScalarizeMaskedMemIntrin for unaligned access expansion.

It already will piggyback on ScalarizeMaskedMemIntrin for scalarization, so that should be straightforward to put into the followup VP-mem-expansion patch.

[simoll]

I am ready to LGTM this. Any blockers?

[craig.topper]

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

1369–1370

Was this idiomatic comment referring to my comment or the switch suggestion?

If we don't do a switch I'd like to see it back the way it was originally so that each opcode is on one line. We can put clang-format off/on around it to suppress the clang format warning.

llvm/include/llvm/IR/IntrinsicInst.h

428

This should return unsigned to match the recent change to GetVectorLengthParamPos and GetMaskParamPos.

432

unsigned

llvm/lib/IR/IntrinsicInst.cpp

346

unsigned

[hussainjk]

addressing review comments

[hussainjk]

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

1369–1370

Regarding the switch, but I've gone ahead and changed it to a switch statement.

[craig.topper]

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

1395

Extra space after case here

[frasercrmck]

LGTM other than the last few nits.

llvm/lib/IR/IntrinsicInst.cpp

348

Is the explicit this-> required?

[craig.topper]

LGTM too. Other than the last comments from Fraser and I.

[hussainjk]

addressing review comments

[hussainjk]

I believe I've addressed everything now.

[craig.topper]

LGTM

[hussainjk]

I don't have commit access. Can someone commit this please?

[simoll]

Sure. I will commit this.

[simoll]

This needs to be rebased onto recent upstream.
In order to pass all unittest then, this patch needs an implementation in VPIntrinsic::getDeclarationForParams.

[frasercrmck]

Just so I know for the VP reduction intrinsics, should we not be adding LangRef documentation at this stage too, given we're adding new intrinsics? How about support in the ExpandVectorPredication pass? Should that come now or can it wait?

[simoll]

In D99355#2816492, @frasercrmck wrote:

Just so I know for the VP reduction intrinsics, should we not be adding LangRef documentation at this stage too, given we're adding new intrinsics? How about support in the ExpandVectorPredication pass? Should that come now or can it wait?

Ideally, yes, you want to have the documentation with the intrinsics and their expansion in one patch.
If that grows too large, you split off the expansion for another patch.
For this patch, since we are kind of stalling on memory intrinsics to become available upstream, it'd be ok for me to just have the intrinsics and save everything else for followup patches. The intrinsics are documented in the reference implementation anyway.

[frasercrmck]

In D99355#2818618, @simoll wrote:

In D99355#2816492, @frasercrmck wrote:

Just so I know for the VP reduction intrinsics, should we not be adding LangRef documentation at this stage too, given we're adding new intrinsics? How about support in the ExpandVectorPredication pass? Should that come now or can it wait?

Ideally, yes, you want to have the documentation with the intrinsics and their expansion in one patch.
If that grows too large, you split off the expansion for another patch.
For this patch, since we are kind of stalling on memory intrinsics to become available upstream, it'd be ok for me to just have the intrinsics and save everything else for followup patches. The intrinsics are documented in the reference implementation anyway.

Got you, thanks for the info. I'd definitely prefer to have it all done in one for my own patches but I'm not going to block this one or anything.

[hussainjk]

rebasing

[hussainjk]

rebasing

[hussainjk]

I rebased, fixed some failing unit tests, and extended VPIntrinsic::getDeclarationForParams to handle memory intrins. Not sure if I got arc to properly pick everything up.

[simoll]

Apart from a style nit, LGTM. Will run some local tests before accepting. Thx!

llvm/unittests/IR/VPIntrinsicTest.cpp

249

No braces

[simoll]

One more thing: title & summary do not reflect what's in the patch anymore. Please update those too.

[hussainjk]

addressing review comments

[hussainjk]

In D99355#2849287, @simoll wrote:

One more thing: title & summary do not reflect what's in the patch anymore. Please update those too.

Ahh, can you clarify?

[simoll]

In D99355#2850296, @hussainjk wrote:

In D99355#2849287, @simoll wrote:

One more thing: title & summary do not reflect what's in the patch anymore. Please update those too.

Ahh, can you clarify?

My bad. I had overlooked that the VPIntrinsics.def implicitly instantiates the SDNodes. (I thought for a bit there were no SDNodes in this patch anymore). You're all good :)

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

const SDValue &getBasePtr() const {
     default:
       return getOperand(1);
     }
   }
 
   // Methods to support isa and dyn_cast
   static bool classof(const SDNode *N) {
     // For some targets, we lower some target intrinsics to a MemIntrinsicNode
     // with either an intrinsic or a target opcode.
-    return N->getOpcode() == ISD::LOAD                ||
-           N->getOpcode() == ISD::STORE               ||
-           N->getOpcode() == ISD::PREFETCH            ||
-           N->getOpcode() == ISD::ATOMIC_CMP_SWAP     ||
-           N->getOpcode() == ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS ||
-           N->getOpcode() == ISD::ATOMIC_SWAP         ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_ADD     ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_SUB     ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_AND     ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_CLR     ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_OR      ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_XOR     ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_NAND    ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_MIN     ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_MAX     ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_UMIN    ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_UMAX    ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_FADD    ||
-           N->getOpcode() == ISD::ATOMIC_LOAD_FSUB    ||
-           N->getOpcode() == ISD::ATOMIC_LOAD         ||
-           N->getOpcode() == ISD::ATOMIC_STORE        ||
-           N->getOpcode() == ISD::MLOAD               ||
-           N->getOpcode() == ISD::MSTORE              ||
-           N->getOpcode() == ISD::MGATHER             ||
-           N->getOpcode() == ISD::MSCATTER            ||
-           N->isMemIntrinsic()                        ||
-           N->isTargetMemoryOpcode();
+    switch (N->getOpcode()) {
+    case ISD::LOAD:
+    case ISD::STORE:
+    case ISD::PREFETCH:
+    case ISD::ATOMIC_CMP_SWAP:
+    case ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS:
+    case ISD::ATOMIC_SWAP:
+    case ISD::ATOMIC_LOAD_ADD:
+    case ISD::ATOMIC_LOAD_SUB:
+    case ISD::ATOMIC_LOAD_AND:
+    case ISD::ATOMIC_LOAD_CLR:
+    case ISD::ATOMIC_LOAD_OR:
+    case ISD::ATOMIC_LOAD_XOR:
+    case ISD::ATOMIC_LOAD_NAND:
+    case ISD::ATOMIC_LOAD_MIN:
+    case ISD::ATOMIC_LOAD_MAX:
+    case ISD::ATOMIC_LOAD_UMIN:
+    case ISD::ATOMIC_LOAD_UMAX:
+    case ISD::ATOMIC_LOAD_FADD:
+    case ISD::ATOMIC_LOAD_FSUB:
+    case ISD::ATOMIC_LOAD:
+    case ISD::ATOMIC_STORE:
+    case ISD::MLOAD:
+    case ISD::MSTORE:
+    case ISD::MGATHER:
+    case ISD::MSCATTER:
+      return true;
+    default:
+      return N->isMemIntrinsic() || N->isTargetMemoryOpcode();
+    }
   }
 };
 
 /// This is an SDNode representing atomic operations.
 class AtomicSDNode : public MemSDNode {
 public:
   AtomicSDNode(unsigned Opc, unsigned Order, const DebugLoc &dl, SDVTList VTL,
                EVT MemVT, MachineMemOperand *MMO)

llvm/include/llvm/IR/IntrinsicInst.h

public:
   static Optional<unsigned> getVectorLengthParamPos(Intrinsic::ID IntrinsicID);
 
   /// The llvm.vp.* intrinsics for this instruction Opcode
   static Intrinsic::ID getForOpcode(unsigned OC);
 
   // Whether \p ID is a VP intrinsic ID.
   static bool isVPIntrinsic(Intrinsic::ID);
 
-  /// \return the mask parameter or nullptr.
+  /// \return The mask parameter or nullptr.
   Value *getMaskParam() const;
   void setMaskParam(Value *);
 
-  /// \return the vector length parameter or nullptr.
+  /// \return The vector length parameter or nullptr.
   Value *getVectorLengthParam() const;
   void setVectorLengthParam(Value *);
 
-  /// \return whether the vector length param can be ignored.
+  /// \return Whether the vector length param can be ignored.
   bool canIgnoreVectorLengthParam() const;
 
-  /// \return the static element count (vector number of elements) the vector
+  /// \return The static element count (vector number of elements) the vector
   /// length parameter applies to.
   ElementCount getStaticVectorLength() const;
 
+  /// \return The alignment of the pointer used by this load/store/gather or
+  /// scatter.
+  MaybeAlign getPointerAlignment() const;
+  // MaybeAlign setPointerAlignment(Align NewAlign); // TODO
+
+  /// \return The pointer operand of this load,store, gather or scatter.
+  Value *getMemoryPointerParam() const;
+  static Optional<unsigned> getMemoryPointerParamPos(Intrinsic::ID);
+
+  /// \return The data (payload) operand of this store or scatter.
+  Value *getMemoryDataParam() const;
+  static Optional<unsigned> getMemoryDataParamPos(Intrinsic::ID);
+
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const IntrinsicInst *I) {
     return isVPIntrinsic(I->getIntrinsicID());
   }
   static bool classof(const Value *V) {
     return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
   }
 

llvm/include/llvm/IR/Intrinsics.td

 // Intrinsic to mask out bits of a pointer.
 def int_ptrmask: DefaultAttrsIntrinsic<[llvm_anyptr_ty], [LLVMMatchType<0>, llvm_anyint_ty],
                            [IntrNoMem, IntrSpeculatable, IntrWillReturn]>;
 
 def int_experimental_stepvector : DefaultAttrsIntrinsic<[llvm_anyvector_ty],
                                                         [], [IntrNoMem]>;
 
 //===---------------- Vector Predication Intrinsics --------------===//
+// Memory Intrinsics
+def int_vp_store : DefaultAttrsIntrinsic<[],
+                             [ llvm_anyvector_ty,
+                               LLVMAnyPointerType<LLVMMatchType<0>>,
+                               LLVMScalarOrSameVectorWidth<0, llvm_i1_ty>,
+                               llvm_i32_ty],
+                             [ NoCapture<ArgIndex<1>>, IntrNoSync, IntrWriteMem, IntrArgMemOnly, IntrWillReturn ]>;
+
+def int_vp_load  : DefaultAttrsIntrinsic<[ llvm_anyvector_ty],
+                             [ LLVMAnyPointerType<LLVMMatchType<0>>,
+                               LLVMScalarOrSameVectorWidth<0, llvm_i1_ty>,
+                               llvm_i32_ty],
+                             [ NoCapture<ArgIndex<0>>, IntrNoSync, IntrReadMem, IntrWillReturn, IntrArgMemOnly ]>;
+
+def int_vp_gather: DefaultAttrsIntrinsic<[ llvm_anyvector_ty],
+                             [ LLVMVectorOfAnyPointersToElt<0>,
+                               LLVMScalarOrSameVectorWidth<0, llvm_i1_ty>,
+                               llvm_i32_ty],
+                             [ IntrReadMem, IntrNoSync, IntrWillReturn, IntrArgMemOnly ]>;
+
+def int_vp_scatter: DefaultAttrsIntrinsic<[],
+                              [ llvm_anyvector_ty,
+                                LLVMVectorOfAnyPointersToElt<0>,
+                                LLVMScalarOrSameVectorWidth<0, llvm_i1_ty>,
+                                llvm_i32_ty],
+                              [ IntrArgMemOnly, IntrNoSync, IntrWillReturn ]>; // TODO allow IntrNoCapture for vectors of pointers
 
 // Speculatable Binary operators
 let IntrProperties = [IntrSpeculatable, IntrNoMem, IntrNoSync, IntrWillReturn] in {
   def int_vp_add : DefaultAttrsIntrinsic<[ llvm_anyvector_ty ],
                              [ LLVMMatchType<0>,
                                LLVMMatchType<0>,
                                LLVMScalarOrSameVectorWidth<0, llvm_i1_ty>,
                                llvm_i32_ty]>;
   def int_vp_sub : DefaultAttrsIntrinsic<[ llvm_anyvector_ty ],

llvm/include/llvm/IR/VPIntrinsics.def

 // Property Macros {
 
 // The intrinsic and/or SDNode has the same function as this LLVM IR Opcode.
 // \p OPC  The standard IR opcode.
 #ifndef HANDLE_VP_TO_OPC
 #define HANDLE_VP_TO_OPC(OPC)
 #endif
 
 // Whether the intrinsic may have a rounding mode or exception behavior operand
 // bundle.
 // \p HASROUND   '1' if the intrinsic can have a rounding mode operand bundle,
 //               '0' otherwise.
 // \p HASEXCEPT  '1' if the intrinsic can have an exception behavior operand
 //               bundle, '0' otherwise.
 // \p INTRINID  The constrained fp intrinsic this VP intrinsic corresponds to.
 #ifndef HANDLE_VP_TO_CONSTRAINEDFP
 #define HANDLE_VP_TO_CONSTRAINEDFP(HASROUND, HASEXCEPT, INTRINID)
 #endif
 
+// Map this VP intrinsic to its canonical functional intrinsic.
+#ifndef HANDLE_VP_TO_INTRIN
+#define HANDLE_VP_TO_INTRIN(ID)
+#endif
+
+// This VP Intrinsic is a memory operation
+// The pointer arg is at POINTERPOS and the data arg is at DATAPOS.
+#ifndef HANDLE_VP_IS_MEMOP
+#define HANDLE_VP_IS_MEMOP(VPID, POINTERPOS, DATAPOS)
+#endif
+
 /// } Property Macros
 
 ///// Integer Arithmetic {
 
 // Specialized helper macro for integer binary operators (%x, %y, %mask, %evl).
 #ifdef HELPER_REGISTER_BINARY_INT_VP
 #error "The internal helper macro HELPER_REGISTER_BINARY_INT_VP is already defined!"
 #endif

 
 // llvm.vp.frem(x,y,mask,vlen)
 HELPER_REGISTER_BINARY_FP_VP(frem, VP_FREM, FRem)
 
 #undef HELPER_REGISTER_BINARY_FP_VP
 
 ///// } Floating-Point Arithmetic
 
+///// Memory Operations {
+// llvm.vp.store(ptr,val,mask,vlen)
+BEGIN_REGISTER_VP(vp_store, 2, 3, VP_STORE, 0)
+HANDLE_VP_TO_OPC(Store)
+HANDLE_VP_TO_INTRIN(masked_store)
+HANDLE_VP_IS_MEMOP(vp_store, 1, 0)
+END_REGISTER_VP(vp_store, VP_STORE)
+
+// llvm.vp.scatter(ptr,val,mask,vlen)
+BEGIN_REGISTER_VP(vp_scatter, 2, 3, VP_SCATTER, 0)
+HANDLE_VP_TO_INTRIN(masked_scatter)
+HANDLE_VP_IS_MEMOP(vp_scatter, 1, 0)
+END_REGISTER_VP(vp_scatter, VP_SCATTER)
+
+// llvm.vp.load(ptr,mask,vlen)
+BEGIN_REGISTER_VP(vp_load, 1, 2, VP_LOAD, -1)
+HANDLE_VP_TO_OPC(Load)
+HANDLE_VP_TO_INTRIN(masked_load)
+HANDLE_VP_IS_MEMOP(vp_load, 0, None)
+END_REGISTER_VP(vp_load, VP_LOAD)
+
+// llvm.vp.gather(ptr,mask,vlen)
+BEGIN_REGISTER_VP(vp_gather, 1, 2, VP_GATHER, -1)
+HANDLE_VP_TO_INTRIN(masked_gather)
+HANDLE_VP_IS_MEMOP(vp_gather, 0, None)
+END_REGISTER_VP(vp_gather, VP_GATHER)
+
+///// } Memory Operations
+
+
 #undef BEGIN_REGISTER_VP
 #undef BEGIN_REGISTER_VP_INTRINSIC
 #undef BEGIN_REGISTER_VP_SDNODE
 #undef END_REGISTER_VP
 #undef END_REGISTER_VP_INTRINSIC
 #undef END_REGISTER_VP_SDNODE
 #undef HANDLE_VP_TO_OPC
 #undef HANDLE_VP_TO_CONSTRAINEDFP
+#undef HANDLE_VP_TO_INTRIN
+#undef HANDLE_VP_IS_MEMOP

llvm/lib/IR/IntrinsicInst.cpp

-//===-- InstrinsicInst.cpp - Intrinsic Instruction Wrappers ---------------===//
+//===-- IntrinsicInst.cpp - Intrinsic Instruction Wrappers ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements methods that make it really easy to deal with intrinsic

 
 #define BEGIN_REGISTER_VP_INTRINSIC(VPID, MASKPOS, VLENPOS)                    \
   case Intrinsic::VPID:                                                        \
     return VLENPOS;
 #include "llvm/IR/VPIntrinsics.def"
   }
 }
 
+/// \return the alignment of the pointer used by this load/store/gather or
+/// scatter.
+MaybeAlign VPIntrinsic::getPointerAlignment() const {
+  Optional<unsigned> PtrParamOpt = getMemoryPointerParamPos(getIntrinsicID());
+  assert(PtrParamOpt.hasValue() && "no pointer argument!");
+  return getParamAlign(PtrParamOpt.getValue());
+}
+
+/// \return The pointer operand of this load,store, gather or scatter.
+Value *VPIntrinsic::getMemoryPointerParam() const {
+  if (auto PtrParamOpt = getMemoryPointerParamPos(getIntrinsicID()))
+    return getArgOperand(PtrParamOpt.getValue());
+  return nullptr;
+}
+
+Optional<unsigned> VPIntrinsic::getMemoryPointerParamPos(Intrinsic::ID VPID) {
+  switch (VPID) {
+  default:
+    return None;
+
+#define HANDLE_VP_IS_MEMOP(VPID, POINTERPOS, DATAPOS)                          \
+  case Intrinsic::VPID:                                                        \
+    return POINTERPOS;
+#include "llvm/IR/VPIntrinsics.def"
+  }
+}
+
+/// \return The data (payload) operand of this store or scatter.
+Value *VPIntrinsic::getMemoryDataParam() const {
+  auto DataParamOpt = getMemoryDataParamPos(getIntrinsicID());
+  if (!DataParamOpt.hasValue())
+    return nullptr;
+  return getArgOperand(DataParamOpt.getValue());
+}
+
+Optional<unsigned> VPIntrinsic::getMemoryDataParamPos(Intrinsic::ID VPID) {
+  switch (VPID) {
+  default:
+    return None;
+
+#define HANDLE_VP_IS_MEMOP(VPID, POINTERPOS, DATAPOS)                          \
+  case Intrinsic::VPID:                                                        \
+    return DATAPOS;
+#include "llvm/IR/VPIntrinsics.def"
+  }
+}
+
 bool VPIntrinsic::isVPIntrinsic(Intrinsic::ID ID) {
   switch (ID) {
   default:
     return false;
 
 #define BEGIN_REGISTER_VP_INTRINSIC(VPID, MASKPOS, VLENPOS)                    \
   case Intrinsic::VPID:                                                        \
     break;
if (VLNum >= EC.getKnownMinValue())
     return true;
 
   return false;
 }
 
 Function *VPIntrinsic::getDeclarationForParams(Module *M, Intrinsic::ID VPID,
                                                ArrayRef<Value *> Params) {
   assert(isVPIntrinsic(VPID) && "not a VP intrinsic");
-
-  // TODO: Extend this for other VP intrinsics as they are upstreamed. This
-  // works for binary arithmetic VP intrinsics.
-  auto *VPFunc = Intrinsic::getDeclaration(M, VPID, Params[0]->getType());
+  Function *VPFunc;
+  switch (VPID) {
+  default:
+    VPFunc = Intrinsic::getDeclaration(M, VPID, Params[0]->getType());
+    break;
+  case Intrinsic::vp_load:
+    VPFunc = Intrinsic::getDeclaration(
+        M, VPID,
+        {Params[0]->getType()->getPointerElementType(), Params[0]->getType()});
+    break;
+  case Intrinsic::vp_gather:
+    VPFunc = Intrinsic::getDeclaration(
+        M, VPID,
+        {VectorType::get(cast<VectorType>(Params[0]->getType())
+                             ->getElementType()
+                             ->getPointerElementType(),
+                         cast<VectorType>(Params[0]->getType())),
+         Params[0]->getType()});
+    break;
+  case Intrinsic::vp_store:
+    VPFunc = Intrinsic::getDeclaration(
+        M, VPID,
+        {Params[1]->getType()->getPointerElementType(), Params[1]->getType()});
+    break;
+  case Intrinsic::vp_scatter:
+    VPFunc = Intrinsic::getDeclaration(
+        M, VPID, {Params[0]->getType(), Params[1]->getType()});
+    break;
+  }
   assert(VPFunc && "Could not declare VP intrinsic");
   return VPFunc;
 }
 
 Instruction::BinaryOps BinaryOpIntrinsic::getBinaryOp() const {
   switch (getIntrinsicID()) {
   case Intrinsic::uadd_with_overflow:
   case Intrinsic::sadd_with_overflow:

llvm/unittests/IR/VPIntrinsicTest.cpp

 class VPIntrinsicTest : public testing::Test {
 protected:
   LLVMContext Context;
 
   VPIntrinsicTest() : Context() {}
 
   LLVMContext C;
   SMDiagnostic Err;
 
   std::unique_ptr<Module> createVPDeclarationModule() {
     const char *BinaryIntOpcodes[] = {"add",  "sub",  "mul", "sdiv", "srem",
                                       "udiv", "urem", "and", "xor",  "or",
                                       "ashr", "lshr", "shl"};
     std::stringstream Str;
     for (const char *BinaryIntOpcode : BinaryIntOpcodes)
       Str << " declare <8 x i32> @llvm.vp." << BinaryIntOpcode
           << ".v8i32(<8 x i32>, <8 x i32>, <8 x i1>, i32) ";
 
     const char *BinaryFPOpcodes[] = {"fadd", "fsub", "fmul", "fdiv", "frem"};
     for (const char *BinaryFPOpcode : BinaryFPOpcodes)
       Str << " declare <8 x float> @llvm.vp." << BinaryFPOpcode
           << ".v8f32(<8 x float>, <8 x float>, <8 x i1>, i32) ";
 
+    Str << " declare void @llvm.vp.store.v8i32.p0v8i32(<8 x i32>, <8 x i32>*, <8 x i1>, i32) ";
+        Str << " declare void @llvm.vp.scatter.v8i32.v8p0i32(<8 x i32>, <8 x i32*>, <8 x i1>, i32) ";
+        Str << " declare <8 x i32> @llvm.vp.load.v8i32.p0v8i32(<8 x i32>*, <8 x i1>, i32) ";
+        Str << " declare <8 x i32> @llvm.vp.gather.v8i32.v8p0i32(<8 x i32*>, <8 x i1>, i32) ";
+
     return parseAssemblyString(Str.str(), Err, C);
   }
 };
 
 /// Check that the property scopes include/llvm/IR/VPIntrinsics.def are closed.
 TEST_F(VPIntrinsicTest, VPIntrinsicsDefScopes) {
   Optional<Intrinsic::ID> ScopeVPID;
 #define BEGIN_REGISTER_VP_INTRINSIC(VPID, ...)                                 \
   ASSERT_FALSE(ScopeVPID.hasValue());                                          \
TEST_F(VPIntrinsicTest, VPIntrinsicDeclarationForParams) {
   auto OutM = std::make_unique<Module>("", M->getContext());
 
   for (auto &F : *M) {
     auto *FuncTy = F.getFunctionType();
 
     // Declare intrinsic anew with explicit types.
     std::vector<Value *> Values;
     for (auto *ParamTy : FuncTy->params())
       Values.push_back(UndefValue::get(ParamTy));
 
     ASSERT_NE(F.getIntrinsicID(), Intrinsic::not_intrinsic);
     auto *NewDecl = VPIntrinsic::getDeclarationForParams(
         OutM.get(), F.getIntrinsicID(), Values);
     ASSERT_TRUE(NewDecl);
 
     // Check that 'old decl' == 'new decl'.
     ASSERT_EQ(F.getIntrinsicID(), NewDecl->getIntrinsicID());