[PATCH] D53011: [LV] Add support for vectorizing predicated strided accesses using masked interleave-group
Ayal Zaks via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 9 04:05:22 PDT 2018
Ayal added inline comments.
================
Comment at: include/llvm/Analysis/VectorUtils.h:131
+/// This function creates a shuffle mask for duplicating each of the elements
+/// in a vector of \p VF elements \p InterleaveFactor times. It can be used to
+/// transform a mask of \p VF elements into a mask of
----------------
"each of the elements in a vector of \p VF elements" >> “each element in a vector of \p VF elements”, or “each of the \p VF elements in a vector”.
Indicate “Interleaving” in the name of the method, given the InterleaveFactor argument?
================
Comment at: lib/Analysis/VectorUtils.cpp:822
+ // All members of a predicated interleave-group must have the same predicate,
+ // and currently must reside in the same BB.
+ if ((isPredicated(A->getParent()) || isPredicated(B->getParent())) &&
----------------
Below might look simpler using BlockA = A->getParent(); BlockB = B->getParent();
================
Comment at: lib/Target/X86/X86TargetTransformInfo.cpp:2598
DL.getPointerSizeInBits() : ScalarTy->getPrimitiveSizeInBits();
-
return ((DataWidth == 32 || DataWidth == 64) && ST->hasAVX()) ||
----------------
(Unrelated clang-format change)
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:416
+ /// Try to vectorize the interleaved access group that \p Instr belongs to,
+ /// optionally masking the vector operations if \p BlockInMask is non-null.
+ void vectorizeInterleaveGroup(Instruction *Instr,
----------------
" optionally" >> "optionally"
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1985
+ assert ((!Group->isReverse() || !BlockInMask) && "Reversed Masked Interleave "
+ "group not supported. ");
+
----------------
May be slightly better to check (!BlockInMask || !Group->isReverse()); or fold under the "if (IsMaskRequired)" below.
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2041
+ createTransposedMask(Builder, InterleaveFactor, VF),
+ "interleaved.mask");
+ NewLoad = Builder.CreateMaskedLoad(NewPtrs[Part], Group->getAlignment(),
----------------
Above may look simpler by first setting
```
auto *Undefs = UndefValue::get(Mask->getType());
auto *TransposedMask = createTransposedMask(Builder, InterleaveFactor, VF);
```
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2043
+ NewLoad = Builder.CreateMaskedLoad(NewPtrs[Part], Group->getAlignment(),
+ ShuffledMask, UndefValue::get(VecTy), "wide.masked.load");
+ }
----------------
"wide.masked.load" >> "wide.masked.vec", to follow "wide.vec" more closely?
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2120
+ createTransposedMask(Builder, InterleaveFactor, VF),
+ "interleaved.mask");
+ NewStoreInstr = Builder.CreateMaskedStore(IVec, NewPtrs[Part],
----------------
See above simplification comment.
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4304
+ ? (TTI.isLegalMaskedLoad(Ty) || isLegalMaskedGather(Ty))
+ : (TTI.isLegalMaskedStore(Ty) || isLegalMaskedScatter(Ty));
+}
----------------
Better check only if `isLegalMaskedLoad` : `isLegalMaskedStore`. The purpose of optimizing interleaved accesses is to improve upon the use of Gather/Scatter.
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7108
+ bool UseMaskedInterleaved = useMaskedInterleavedAccesses(*TTI);
+ IAI.analyzeInterleaving(UseMaskedInterleaved);
}
----------------
Simpler to fold into
```
IAI.analyzeInterleaving(useMaskedInterleavedAccesses(*TTI));
```
https://reviews.llvm.org/D53011
More information about the llvm-commits
mailing list