[PATCH] D147467: [VPlan] Add VPInterleaveRecipe::NeedsMaskForGaps field (NFCI).

[Ayal Zaks via Phabricator via llvm-commits]

Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

Whether masking of suffix gaps in interleaved loads can be optimized away or not should indeed be recorded during planning rather than IR generation.
Adding a couple of nits.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2674
 
   Value *MaskForGaps = nullptr;
+  if (NeedsMaskForGaps) {
----------------
nit (Independent of this patch): MaskForGaps is set here for loads and later reset for stores. Better to either set it here for both or place this setting inside the handling of loads.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9028
+        IG, Recipe->getAddr(), StoredValues, Recipe->getMask(),
+        IG->requiresScalarEpilogue() && !CM.isScalarEpilogueAllowed());
     VPIG->insertBefore(Recipe);
----------------
nit: may be clearer to first set `bool NeedsMaskForGaps = IG->requiresScalarEpilogue() && !CM.isScalarEpilogueAllowed();` and then use it, instead of setting it inline.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1396
 
+  bool NeedsMaskForGaps = false;
+
----------------
nit: would be good to document and clarify or rename the two masks: `HasMask` indicates if the interleave group is inside a conditional basic block, i.e., HasBlockMask or HasMaskForBlock. `NeedsMaskForGaps` indicates if the interleave group of loads is allowed to speculatively load absent unused members or must avoid doing so by using a mask, i.e., !CanSpeculativelyLoadWithoutMask, where the speculation relies on loading present members on both sides of missing ones - gaps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147467