Skip to content

[Transform][LoadStoreVectorizer] allow redundant in Chain #163019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Nov 13, 2025
Merged

[Transform][LoadStoreVectorizer] allow redundant in Chain #163019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
74cdc65
[Transform][LoadStoreVectorizer] allow redundant in Chain
cmc-rep Oct 11, 2025
b5a2f3d
[LoadStoreVectorizer] Fix tests
cmc-rep Oct 12, 2025
0286f06
[LoadStoreVectorizer] Fix more tests
cmc-rep Oct 12, 2025
22d061e
[AMDGPU] Update a test using tool-generated check according to review
cmc-rep Oct 14, 2025
9bba9c7
[Transform][LoadStoreVectorizer] Allow partial redundant
cmc-rep Oct 15, 2025
c93bad9
[Transform][LoadStoreVectorizer] fix tests after allowing partial red…
cmc-rep Oct 15, 2025
05bb268
[Transform][LoadStoreVectorizer] move redundancy calculation to Vecto…
cmc-rep Oct 17, 2025
72d2db1
[Transform][LoadStoreVectorizer] fix merging result of one-element ve…
cmc-rep Oct 18, 2025
57aa3b6
followup fixing tests
cmc-rep Oct 18, 2025
6ef6230
[LoadStoreVectorizer] review update
cmc-rep Nov 13, 2025
052ed61
[LoadStoreVectorizer] update test after rebase
cmc-rep Nov 13, 2025
ac27f5f
[LoadStoreVectorizer] More review updates
cmc-rep Nov 13, 2025
76de96d
[LoadStoreVectorizer] Rebase and update test.
cmc-rep Nov 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 50 additions & 26 deletions llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,26 +626,35 @@ std::vector<Chain> Vectorizer::splitChainByContiguity(Chain &C) {
std::vector<Chain> Ret;
Ret.push_back({C.front()});

unsigned ElemBytes = DL.getTypeStoreSize(getChainElemTy(C));
APInt PrevReadEnd = C[0].OffsetFromLeader +
DL.getTypeStoreSize(getLoadStoreType(&*C[0].Inst));
for (auto It = std::next(C.begin()), End = C.end(); It != End; ++It) {
// `prev` accesses offsets [PrevDistFromBase, PrevReadEnd).
auto &CurChain = Ret.back();
const ChainElem &Prev = CurChain.back();
unsigned SzBits = DL.getTypeSizeInBits(getLoadStoreType(&*Prev.Inst));
assert(SzBits % 8 == 0 && "Non-byte sizes should have been filtered out by "
"collectEquivalenceClass");
APInt PrevReadEnd = Prev.OffsetFromLeader + SzBits / 8;
unsigned SzBytes = DL.getTypeStoreSize(getLoadStoreType(&*It->Inst));

// Add this instruction to the end of the current chain, or start a new one.
bool AreContiguous = It->OffsetFromLeader == PrevReadEnd;
LLVM_DEBUG(dbgs() << "LSV: Instructions are "
<< (AreContiguous ? "" : "not ") << "contiguous: "
<< *Prev.Inst << " (ends at offset " << PrevReadEnd
<< ") -> " << *It->Inst << " (starts at offset "
assert(SzBytes % ElemBytes == 0);
APInt ReadEnd = It->OffsetFromLeader + SzBytes;
// Allow redundancy: partial or full overlap counts as contiguous.
bool AreContiguous = false;
if (It->OffsetFromLeader.sle(PrevReadEnd)) {
uint64_t Overlap = (PrevReadEnd - It->OffsetFromLeader).getZExtValue();
if (Overlap % ElemBytes == 0)
AreContiguous = true;
}

LLVM_DEBUG(dbgs() << "LSV: Instruction is "
<< (AreContiguous ? "contiguous" : "chain-breaker")
<< *It->Inst << " (starts at offset "
<< It->OffsetFromLeader << ")\n");

if (AreContiguous)
CurChain.push_back(*It);
else
Ret.push_back({*It});
PrevReadEnd = APIntOps::smax(PrevReadEnd, ReadEnd);
}

// Filter out length-1 chains, these are uninteresting.
Expand Down Expand Up @@ -874,15 +883,24 @@ bool Vectorizer::vectorizeChain(Chain &C) {
Type *VecElemTy = getChainElemTy(C);
bool IsLoadChain = isa<LoadInst>(C[0].Inst);
unsigned AS = getLoadStoreAddressSpace(C[0].Inst);
unsigned ChainBytes = std::accumulate(
C.begin(), C.end(), 0u, [&](unsigned Bytes, const ChainElem &E) {
return Bytes + DL.getTypeStoreSize(getLoadStoreType(E.Inst));
});
unsigned BytesAdded = DL.getTypeStoreSize(getLoadStoreType(&*C[0].Inst));
APInt PrevReadEnd = C[0].OffsetFromLeader + BytesAdded;
unsigned ChainBytes = BytesAdded;
for (auto It = std::next(C.begin()), End = C.end(); It != End; ++It) {
unsigned SzBytes = DL.getTypeStoreSize(getLoadStoreType(&*It->Inst));
APInt ReadEnd = It->OffsetFromLeader + SzBytes;
// Update ChainBytes considering possible overlap.
BytesAdded =
PrevReadEnd.sle(ReadEnd) ? (ReadEnd - PrevReadEnd).getSExtValue() : 0;
ChainBytes += BytesAdded;
PrevReadEnd = APIntOps::smax(PrevReadEnd, ReadEnd);
}

assert(ChainBytes % DL.getTypeStoreSize(VecElemTy) == 0);
// VecTy is a power of 2 and 1 byte at smallest, but VecElemTy may be smaller
// than 1 byte (e.g. VecTy == <32 x i1>).
Type *VecTy = FixedVectorType::get(
VecElemTy, 8 * ChainBytes / DL.getTypeSizeInBits(VecElemTy));
unsigned NumElem = 8 * ChainBytes / DL.getTypeSizeInBits(VecElemTy);
Type *VecTy = FixedVectorType::get(VecElemTy, NumElem);

Align Alignment = getLoadStoreAlignment(C[0].Inst);
// If this is a load/store of an alloca, we might have upgraded the alloca's
Expand All @@ -909,27 +927,31 @@ bool Vectorizer::vectorizeChain(Chain &C) {
llvm::min_element(C, [](const auto &A, const auto &B) {
return A.Inst->comesBefore(B.Inst);
})->Inst);

// This can happen due to a chain of redundant loads.
// In this case, just use the element-type, and avoid ExtractElement.
if (NumElem == 1)
VecTy = VecElemTy;
Comment on lines +930 to +933
Copy link
Collaborator

@nhaehnle nhaehnle Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating the type here in the middle of the code is unfortunate. Better to move it to the top where VecTy is defined.

Copy link
Contributor Author

@cmc-rep cmc-rep Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only want to apply the type change for load-chain. Right now, we perhaps won't see redundancy in store chain due to aliasing check. When that part is improved, we can have the following:
vec1 = insert-element <1 x i32> vec0, v0, 0
vec2 = insert-element <1 x i32> vec1, v1, 0
store <1xi32> vec2, addr
Having those insert-elements preserves the correct write order.

// Chain is in offset order, so C[0] is the instr with the lowest offset,
// i.e. the root of the vector.
VecInst = Builder.CreateAlignedLoad(VecTy,
getLoadStorePointerOperand(C[0].Inst),
Alignment);

unsigned VecIdx = 0;
for (const ChainElem &E : C) {
Instruction *I = E.Inst;
Value *V;
Type *T = getLoadStoreType(I);
int EOffset = (E.OffsetFromLeader - C[0].OffsetFromLeader).getSExtValue();
int VecIdx = 8 * EOffset / DL.getTypeSizeInBits(VecElemTy);
if (auto *VT = dyn_cast<FixedVectorType>(T)) {
auto Mask = llvm::to_vector<8>(
llvm::seq<int>(VecIdx, VecIdx + VT->getNumElements()));
V = Builder.CreateShuffleVector(VecInst, Mask, I->getName());
VecIdx += VT->getNumElements();
} else {
} else if (VecTy != VecElemTy) {
V = Builder.CreateExtractElement(VecInst, Builder.getInt32(VecIdx),
I->getName());
++VecIdx;
} else {
V = VecInst;
}
if (V->getType() != I->getType())
V = Builder.CreateBitOrPointerCast(V, I->getType());
Expand Down Expand Up @@ -964,22 +986,24 @@ bool Vectorizer::vectorizeChain(Chain &C) {

// Build the vector to store.
Value *Vec = PoisonValue::get(VecTy);
unsigned VecIdx = 0;
auto InsertElem = [&](Value *V) {
auto InsertElem = [&](Value *V, unsigned VecIdx) {
if (V->getType() != VecElemTy)
V = Builder.CreateBitOrPointerCast(V, VecElemTy);
Vec = Builder.CreateInsertElement(Vec, V, Builder.getInt32(VecIdx++));
Vec = Builder.CreateInsertElement(Vec, V, Builder.getInt32(VecIdx));
};
for (const ChainElem &E : C) {
auto *I = cast<StoreInst>(E.Inst);
int EOffset = (E.OffsetFromLeader - C[0].OffsetFromLeader).getSExtValue();
int VecIdx = 8 * EOffset / DL.getTypeSizeInBits(VecElemTy);
if (FixedVectorType *VT =
dyn_cast<FixedVectorType>(getLoadStoreType(I))) {
for (int J = 0, JE = VT->getNumElements(); J < JE; ++J) {
InsertElem(Builder.CreateExtractElement(I->getValueOperand(),
Builder.getInt32(J)));
Builder.getInt32(J)),
VecIdx++);
}
} else {
InsertElem(I->getValueOperand());
InsertElem(I->getValueOperand(), VecIdx);
}
}

Expand Down
9 changes: 5 additions & 4 deletions llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-call.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3850,8 +3850,9 @@ define amdgpu_kernel void @test_call_external_void_func_v32i32_p3_p5() #0 {
; CHECK-NEXT: [[DEF1:%[0-9]+]]:_(p1) = G_IMPLICIT_DEF
; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(p1) = G_LOAD [[DEF]](p4) :: (invariant load (p1) from `ptr addrspace(4) poison`, addrspace 4)
; CHECK-NEXT: [[LOAD1:%[0-9]+]]:_(<32 x s32>) = G_LOAD [[LOAD]](p1) :: ("amdgpu-noclobber" load (<32 x s32>) from %ir.ptr0, addrspace 1)
; CHECK-NEXT: [[LOAD2:%[0-9]+]]:_(p3) = G_LOAD [[DEF1]](p1) :: ("amdgpu-noclobber" load (p3) from `ptr addrspace(1) poison`, addrspace 1)
; CHECK-NEXT: [[LOAD3:%[0-9]+]]:_(p5) = G_LOAD [[DEF1]](p1) :: ("amdgpu-noclobber" load (p5) from `ptr addrspace(1) poison`, addrspace 1)
; CHECK-NEXT: [[LOAD2:%[0-9]+]]:_(s32) = G_LOAD [[DEF1]](p1) :: ("amdgpu-noclobber" load (s32) from `ptr addrspace(1) poison`, addrspace 1)
; CHECK-NEXT: [[INTTOPTR:%[0-9]+]]:_(p3) = G_INTTOPTR [[LOAD2]](s32)
; CHECK-NEXT: [[INTTOPTR1:%[0-9]+]]:_(p5) = G_INTTOPTR [[LOAD2]](s32)
; CHECK-NEXT: ADJCALLSTACKUP 0, 0, implicit-def $scc
; CHECK-NEXT: [[GV:%[0-9]+]]:_(p0) = G_GLOBAL_VALUE @external_void_func_v32i32_p3_p5
; CHECK-NEXT: [[COPY10:%[0-9]+]]:_(p4) = COPY [[COPY8]]
Expand Down Expand Up @@ -3880,10 +3881,10 @@ define amdgpu_kernel void @test_call_external_void_func_v32i32_p3_p5() #0 {
; CHECK-NEXT: G_STORE [[UV31]](s32), [[PTR_ADD1]](p5) :: (store (s32) into stack, align 16, addrspace 5)
; CHECK-NEXT: [[C4:%[0-9]+]]:_(s32) = G_CONSTANT i32 4
; CHECK-NEXT: [[PTR_ADD2:%[0-9]+]]:_(p5) = G_PTR_ADD [[AMDGPU_WAVE_ADDRESS]], [[C4]](s32)
; CHECK-NEXT: G_STORE [[LOAD2]](p3), [[PTR_ADD2]](p5) :: (store (p3) into stack + 4, addrspace 5)
; CHECK-NEXT: G_STORE [[INTTOPTR]](p3), [[PTR_ADD2]](p5) :: (store (p3) into stack + 4, addrspace 5)
; CHECK-NEXT: [[C5:%[0-9]+]]:_(s32) = G_CONSTANT i32 8
; CHECK-NEXT: [[PTR_ADD3:%[0-9]+]]:_(p5) = G_PTR_ADD [[AMDGPU_WAVE_ADDRESS]], [[C5]](s32)
; CHECK-NEXT: G_STORE [[LOAD3]](p5), [[PTR_ADD3]](p5) :: (store (p5) into stack + 8, align 8, addrspace 5)
; CHECK-NEXT: G_STORE [[INTTOPTR1]](p5), [[PTR_ADD3]](p5) :: (store (p5) into stack + 8, align 8, addrspace 5)
; CHECK-NEXT: $vgpr0 = COPY [[UV]](s32)
; CHECK-NEXT: $vgpr1 = COPY [[UV1]](s32)
; CHECK-NEXT: $vgpr2 = COPY [[UV2]](s32)
Expand Down
Loading
Loading