[PATCH] D35829: [X86][LLVM]Expanding Supports lowerInterleavedStore() in X86InterleavedAccess (VF16 stride 4).
Guy Blank via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 26 05:05:40 PDT 2017
guyblank added inline comments.
================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:113
+ // 2. 16-element vectors of 8 bits on AVX.
+ // 3. 32-element vectors of 8 bits on AVX.
if (isa<LoadInst>(Inst)) {
----------------
the comment is a bit misleading since it depends on whether we are loading or storing. can you expand the comment to state exactly what is supported?
================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:126
- if (!Subtarget.hasAVX() || Factor != 4 ||
- (ShuffleElemSize != 64 && ShuffleElemSize != 8) ||
+ if (DL.getTypeSizeInBits(ShuffleEltTy) == 8 && isa<StoreInst>(Inst) &&
+ (WideInstSize == 512 || WideInstSize == 1024) && Subtarget.hasAVX() &&
----------------
you can use ShuffleElemSize
================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:127
+ if (DL.getTypeSizeInBits(ShuffleEltTy) == 8 && isa<StoreInst>(Inst) &&
+ (WideInstSize == 512 || WideInstSize == 1024) && Subtarget.hasAVX() &&
+ Factor == 4)
----------------
you can early exit at the beginning of the function on !Subtarget.hasAVX()
================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:132
+ if (!Subtarget.hasAVX() || Factor != 4 || ShuffleElemSize != 64 ||
WideInstSize != (Factor * ShuffleElemSize * SupportedNumElem))
return false;
----------------
so this check isn't needed in the previous if?
Can you try to simplify this function? its getting overly complicated IMO.
================
Comment at: lib/Target/X86/X86InterleavedAccess.cpp:366
break;
+ case 16:
+ interleave8bitStride4(DecomposedVectors, TransposedVectors, MVT::v16i8,
----------------
maybe pass NumSubVecElems to interleave8bitStride4 and have it calculate the MVTs ?
https://reviews.llvm.org/D35829
More information about the llvm-commits
mailing list