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

[Artem Belevich 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));
-  }
----------------
Artem-B wrote:

> I fail to understand why line 837 was not always present. 

Ah. I was commenting on the removed diff changes, and it roughly aligns with your point #1.

As for the line 837, @jlebar would know for sure, as it was done in https://github.com/llvm/llvm-project/commit/2be0abb7fe72ed4537b3eabcd3102d48ea845717

> I fail to understand why line 837 was not always present. 

Do you mean - why the check was not present before the @jlebar's change above? 
The answer is that before the change the pass was scaling extremely poorly with the size of TU. The change improved it a lot by exploiting the fact that virtually all vectorizable memory accesses would be done relative to the same base pointer, so we now analyze only a subset of the related loads/stores and the check on line 837 appears to be another workload shedding heuristic -- if something prevents vectorising particular chain, we can skip it.

Take it with a grain of salt, as I have only vague recollection of the patch. @jlebar may shed more light on it.


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