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

[Lorenzo Albano via Phabricator via llvm-commits]

loralb added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1891
+  bool HiIsEmpty = false;
+  std::tie(LoMemVT, HiMemVT) =
+      DAG.GetDependentSplitDestVTs(SLD->getMemoryVT(), LoVT, &HiIsEmpty);
----------------
craig.topper wrote:
> 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.
I am using GetDependentSplitDestVTs because I need the HiIsEmpty flag, I do not know if there is a better way to do it though.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1895
+  // We need to do this cast of the Stride operand otherwise an illegal
+  // type may appear in the Type-legalized selection DAG.
+  SDValue Ptr = SLD->getBasePtr();
----------------
craig.topper wrote:
> We're inside of type legalization, why wouldn't the stride type get legalized after the split? Though you wouldn't be able to use it in the ISD::MUL below if you didn't extend it first.
I forgot to add the legalization steps for integer operands. I opened a new revision to address this: D123112


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1921
+
+  if (HiIsEmpty) {
+    // The high vp_strided_load has zero storage size. We therefore simply set
----------------
craig.topper wrote:
> Can the HiIsEmpty case happen?
I think that in the case that we have a widening followed by a split it may happen that one of the resulting vectors is empty, similar to what happens for VP_LOAD and VP_STORE. But it also may be that I am misunderstanding how the splitting for strided intrinsics should work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121784