[PATCH] D53668: [LV] Support vectorization of interleave-groups that require an epilog under optsize using masked wide loads

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

Looking at tests next.



================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:833
                                  ArrayRef<unsigned> Indices, unsigned Alignment,
-                                 unsigned AddressSpace, 
-                                 bool IsMasked = false) const;
+                                 unsigned AddressSpace, bool IsMasked = false,
+                                 bool UseMaskForGaps = false) const;
----------------
Maybe rename `IsMasked` into `UseMaskForCond` or `IsConditional`, to distinguish between the two Is/Use Masks.


================
Comment at: include/llvm/Analysis/VectorUtils.h:129
 
+/// Create a mask that masks away gaps of an interleave group.
+///
----------------
"masks away gaps" >> "filters the members"


================
Comment at: include/llvm/Analysis/VectorUtils.h:139
+/// consists of indices).
+Constant *createTrueFalseMaskForGaps(IRBuilder<> &Builder, unsigned VF,
+                                    const InterleaveGroup &Group);
----------------
`createBitMaskForGaps` or `createBinaryMaskForGaps` ?


================
Comment at: include/llvm/Analysis/VectorUtils.h:425
+  /// we optimize for size and don't allow creating a scalar epilogue.
+  void invalidateGroupsRequiringScalarEpilogue(bool EnabledMaskedInterleave);
 
----------------
Check if !EnabledMaskedInterleave before calling invalidateGroups...()?


================
Comment at: lib/Analysis/VectorUtils.cpp:520
+      unsigned HasMember = Group.getMember(j) ? 1 : 0;
+      Mask.push_back(Builder.getInt1(HasMember));
+    }
----------------
Could first "peel" to build a mask for all members, then replicate it VF-1 times. Not sure it's any better.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1960
+static bool useMaskedInterleavedAccesses(const TargetTransformInfo &TTI) {
+  if (!(EnableMaskedInterleavedMemAccesses.getNumOccurrences() > 0))
+    return TTI.enableMaskedInterleavedAccessVectorization();
----------------
More logical to reverse the condition? Admittedly this is only being moved here. 


https://reviews.llvm.org/D53668