[PATCH] D121784: [VP] Add splitting for VP_STRIDED_STORE and VP_STRIDED_LOAD

Mon Mar 21 11:15:50 PDT 2022

craig.topper added inline comments.
Herald added a subscriber: StephenFan.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1891
+  bool HiIsEmpty = false;
+  std::tie(LoMemVT, HiMemVT) =
+      DAG.GetDependentSplitDestVTs(SLD->getMemoryVT(), LoVT, &HiIsEmpty);
----------------
Does MemVT for a VP_STRIDED_LOAD ever mismatch with the element count of the result. This GetDependentSplitDestVTs is because of how we widen masked loads, but we aren't doing that same widening for strided loads.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1921
+
+  if (HiIsEmpty) {
+    // The high vp_strided_load has zero storage size. We therefore simply set
----------------
Can the HiIsEmpty case happen?

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1939
+      MPI = SLD->getPointerInfo().getWithOffset(
+          LoMemVT.getStoreSize().getFixedSize());
+
----------------
Doesn't the offset need to include the stride which you can't do just like you can't for scalable vectors.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121784