[PATCH] D114884: [VP] Strided loads/stores

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/include/llvm/IR/VPIntrinsics.def:217
 ///// Memory Operations {
-// llvm.vp.store(ptr,val,mask,vlen)
+// llvm.vp.store(val,ptr,mask,vlen)
 BEGIN_REGISTER_VP_INTRINSIC(vp_store, 2, 3)
----------------
Is this an existing mistake in the documentation? Can you just push this as an NFC commit? No need to bury it in this patch.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7937
+
+  uint64_t Size = MemoryLocation::getSizeOrUnknown(MemVT.getStoreSize());
+  MachineFunction &MF = getMachineFunction();
----------------
Since this accesses multiple disjoint memory locations, the Size is not MemVT.getStoreSize(). It probably needs to always be unknown. I guess it would be if the stride were statically known to be 0, but not sure it is worth special casing that.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:8070
+  uint64_t Size =
+      MemoryLocation::getSizeOrUnknown(Val.getValueType().getStoreSize());
+  MachineMemOperand *MMO =
----------------
Same as load, the Size can't be based on the store type.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:8126
+  MachineMemOperand *MMO = MF.getMachineMemOperand(
+      PtrInfo, MMOFlags, MemoryLocation::getSizeOrUnknown(SVT.getStoreSize()),
+      Alignment, AAInfo);
----------------
Same


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7451
+      MachinePointerInfo(PtrOperand), MachineMemOperand::MOLoad,
+      VT.getStoreSize().getKnownMinSize(), *Alignment, AAInfo, Ranges);
+
----------------
This Size doesn't make sense. I think it needs to be unknown.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7472
+      MachinePointerInfo(PtrOperand), MachineMemOperand::MOStore,
+      VT.getStoreSize().getKnownMinSize(), *Alignment, AAInfo);
+
----------------
Same


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h:575
+  void visitVPStridedLoad(const VPIntrinsic &VPIntrin, EVT VT,
+                          SmallVector<SDValue, 7> &OpValues);
+  void visitVPStridedStore(const VPIntrinsic &VPIntrin,
----------------
Can these be SmallVectorImpl<SDValue>?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114884