[llvm] [LoadStoreVectorizer] Support vectorization of mixed-type contiguous accesses (PR #177908)

Drew Kersnar via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 1 10:05:18 PDT 2026


================

----------------
dakersnar wrote:

Something is conceptually wrong here now.

This piece of code is relying on the assumption that `DL.getTypeSizeInBits(getChainElemTy(C));` will be invariant throughout the algorithm. In other words, it is assuming that what it gets _now_ for getChainElemTy will be what we end up getting _later_ in vectorizeChain.

With this PR to allow different scalar element types, this is no longer technically true. Imagine a chain like this:

```
// getChainElemTy = i16
load i32 offset 0
load i32 offset 2 // overlapping 2 bytes with the previous load
load i16 offset 6
```
When queried here in this moment, ChainElemTyBits will be 16 (2 bytes), and thus the overlap will be legal and the chain will continue.

Then imagine that the load i16 gets split off of the chain in a future part of the algorithm. It doesn't matter exactly where it happens, just that it _can_ happen.

Now we are left with a chain that looks like this
```
//getChainElemTy = i32
load i32 offset 0
load i32 offset 2 // overlapping 2 bytes with the previous load
```

And then the overlap is suddenly retroactively illegal, and if it makes it to vectorizeChain it will crash.

IMPORTANT TO NOTE: I cannot find a real case that can hit this bug, because splitChainByAlignment won't let non-power-of-2 size chains pass through, but I think we should still make this overlap check more robust to possible future changes, as it is relying on an invariant that is no longer invariant.

Here is the best solution I can come up with:


```
    // Allow redundancy: partial or full overlap counts as contiguous,
    // provided the overlap aligns to the pairwise minimum scalar size of the
    // two elements. This is sufficient because the chain's GCD element size
    // always divides the pairwise minimum (all scalar sizes are powers of 2),
    // and this property is preserved regardless of how later passes split the
    // chain.
    bool AreContiguous = false;
    if (It->OffsetFromLeader.sle(PrevReadEnd)) {
      unsigned PrevScalarBits =
          DL.getTypeSizeInBits(getLoadStoreType(Prev.Inst)->getScalarType());
      unsigned ItScalarBits =
          DL.getTypeSizeInBits(getLoadStoreType(It->Inst)->getScalarType());
      assert(isPowerOf2_32(PrevScalarBits) && isPowerOf2_32(ItScalarBits) &&
             "Pairwise overlap check assumes power-of-2 scalar sizes.");
      uint64_t Overlap = (PrevReadEnd - It->OffsetFromLeader).getZExtValue();
      if (8 * Overlap % std::min(PrevScalarBits, ItScalarBits) == 0)
        AreContiguous = true;
    }
```
If we go with the above, we can now delete the call to getChainElemTy and the assert that uses it. cc @cmc-rep, thoughts?

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


More information about the llvm-commits mailing list