[llvm] [LoadStoreVectorizer] Propagate alignment through contiguous chain (PR #145733)

[Drew Kersnar via llvm-commits]

================
@@ -880,14 +892,6 @@ bool Vectorizer::vectorizeChain(Chain &C) {
       VecElemTy, 8 * ChainBytes / DL.getTypeSizeInBits(VecElemTy));
 
   Align Alignment = getLoadStoreAlignment(C[0].Inst);
-  // If this is a load/store of an alloca, we might have upgraded the alloca's
-  // alignment earlier.  Get the new alignment.
-  if (AS == DL.getAllocaAddrSpace()) {
-    Alignment = std::max(
-        Alignment,
-        getOrEnforceKnownAlignment(getLoadStorePointerOperand(C[0].Inst),
-                                   MaybeAlign(), DL, C[0].Inst, nullptr, &DT));
-  }
----------------
dakersnar wrote:

I'm not sure I follow. Let me elaborate on my understanding. In the trunk version of the code the compiler:
1. upgrades the alignment of allocas in splitChainByAlignment (changing the alloca instruction/IR), then upgrades the _variable representing_ the alignment of the head load/store of the chain, allowing the chain to continue without actually changing the alignment of the load/store instruction.
2. later on, in vectorizeChain, because the alignment was never upgraded on the load/store instruction, it rederives the upgraded alignment from the alloca with a call to getOrEnforceKnownAlignment.

In this subset of my proposed change, the compiler upgrades the alignment on the load/store instruction at the same time it upgrades the alloca alignment (https://github.com/llvm/llvm-project/pull/145733/files#diff-e0eab10050c9ef433cb0d7bc38e32e8d7fe44cdb0cf7422ae7a98966bff53672R837). This prevents needing to rederive that upgraded alignment later on in vectorizeChain (the deleted block this thread is based on).

My main point is that even without the additional alignment propagation introduced in this patch, I fail to understand why line 837 was not always present. I don't see the point of deferring that upgrade, since it is more complicated from a code complexity perspective. I wanted to make sure I wasn't missing some key tradeoff.

https://github.com/llvm/llvm-project/pull/145733