[PATCH] D104750: [LV] Support Interleaved Store Group With Gaps

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

Thanks for following up on D53668 <https://reviews.llvm.org/D53668> (with a gap ;-)! Looks good to me, added some minor comments, mostly about comments.



================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:1178
     // We only scale the cost of loads since interleaved store groups aren't
     // allowed to have gaps.
     if (Opcode == Instruction::Load && VecTySize > VecTyLTSize) {
----------------
The above comment should be updated, along with its consequences...

In some cases, scaling might be able to handle gaps w/o masking, for both loads and stores; perhaps worth leaving behind a TODO.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:1230
       //
       // E.g. An interleaved store of factor 2:
       //      %v0_v1 = shuffle %v0, %v1, <0, 4, 1, 5, 2, 6, 3, 7>
----------------
Update this example to also include gaps, similar to the interleaved load with gaps example above?


================
Comment at: llvm/lib/Analysis/VectorUtils.cpp:1276
+    // stores with gaps, which are implemented with masked-store (rather than
+    // specualtive access, as in loads).
+    if (InvalidateGroupIfMemberMayWrap(Group, 0, std::string("first")))
----------------
typo: specualtive


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2831
+  assert((!MaskForGaps || !VF.isScalable()) &&
+         "scalable vectors not yet supported.");
   for (unsigned Part = 0; Part < UF; Part++) {
----------------
"scalable vectors not yet supported." >>
"masking gaps for scalable vectors is not supported yet."


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/x86-interleaved-store-accesses-with-gaps.ll:17
+;       points[i*4 + 1] = y[i];
+;     }
+
----------------
Can also relate this test case (or file) to PR50566.


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

https://reviews.llvm.org/D104750