[PATCH] D149893: Rewrite LSV to handle longer chains.

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

Thank you for the reviews!



================
Comment at: llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:805
+      // existing tests.  This isn't *so* bad, because at most we align to 4
+      // bytes (current value of StackAdjustedAlignment).
+      //
----------------
barannikov88 wrote:
> Does DL.getStackAlign() instead of 4 cause many negative differences in tests?
> 
DL.getStackAlignment() assert-fails on x86, nvptx, and amdgpu:

llvm::DataLayout::getStackAlignment() const: StackNaturalAlign && "StackNaturalAlign must be defined"

Not sure what's going on, but I don't think this is a windmill I want to tilt at in this patch.


================
Comment at: llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:873
+  // VecTy is a power of 2 and 1 byte at smallest, but VecElemTy may be smaller
+  // than 1 byte (e.g. <32 x i1>).
+  Type *VecTy = FixedVectorType::get(
----------------
barannikov88 wrote:
> VecElemTy can't be a vector type, can it?
> 
Yeah, I meant that VecTy == <32 x i1>.  Tried to clarify the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149893