[PATCH] D152386: [LoadStoreVectorizer] Only upgrade align for alloca

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:805
+      Value *PtrOperand = getLoadStorePointerOperand(C[CBegin].Inst);
+      bool IsAllocaAccess = isa<AllocaInst>(PtrOperand->stripInBoundsOffsets());
       Align Alignment = getLoadStoreAlignment(C[CBegin].Inst);
----------------
jlebar wrote:
> getOrEnforceKnownAlignment will merely stripPointerCasts, which is weaker than stripInBoundsOffsets.  Presumably we should do the same here?
I agree that it makes sense to use the same kind of strip here.

When using stripPointerCasts I got some more diffs in the `@load_alloca16_unknown_offset_align1_i32` test case, so then I changed this to use the stronger stripInBoundsOffsets to make the impact of my fixup smaller.

There is however already a FIXME for that specific test:
```
; FIXME: Although the offset is unknown here, we know it is a multiple
; of the element size, so should still be align 4
```
So I think that we will get back to a state when that FIXME makes sense (when the test show an opportunity to improve something since we do not vectorize the ALIGNED case any longer).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152386