[PATCH] D109377: type legalization for vp_load and vp_store.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added a comment.

@hussainjk are you still working on this or are you taking over, @bmahjour? I might be able to find time if you're unavailable.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1941
+SDValue DAGTypeLegalizer::PromoteIntOp_VP_LOAD(VPLoadSDNode *N, unsigned OpNo) {
+  assert(OpNo >= 3 && "Only know how to promote the mask or length!");
+  EVT DataVT = N->getValueType(0);
----------------
Again, I'm not sure whether we ever need to promote the length - it's always a target-specific legal integer type, isn't it?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:2774
+    // Only the lo vp_store is needed.
+    Res = Lo;
+  } else {
----------------
`return Lo` would simplify this flow a bit


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:4344
+                       WidenVT.getVectorNumElements());
+  Mask = ModifyToType(Mask, WideMaskVT, true);
+
----------------
`true` here is `FillWithZeroes` - I don't think we need to do this for VP widening, since we're passing the original length along, right?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:5223
+                         WideVT.getVectorNumElements());
+    Mask = ModifyToType(Mask, WideMaskVT, true);
+  } else {
----------------
same comment about `FillWithZeroes`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109377