From c57ce512d84654faf47ef59770d876f3f1dc3844 Mon Sep 17 00:00:00 2001 From: "Klochkov, Vyacheslav N" Date: Mon, 6 Jan 2025 14:15:03 -0800 Subject: [PATCH] Fix memory error by reversing the order of two lines in LoadStoreVectorizer.cpp const auto &VecTo = EQClasses[KeyTo] const auto &VecFrom = EQClasses[KeyFrom] The entry for KeyTo may not exist before the request, unlike KeyFrom. Therefore, the vector of instructions for KeyTo must be requested firs to avoid invalidation of the reference to the instructions vector for KeyFrom This commit also slightly aligns the debug printings and adds one more LIT case. --- .../Vectorize/LoadStoreVectorizer.cpp | 20 ++++------ .../X86/massive_indirection.ll | 38 +++++++++++++++++++ 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp b/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp index 9f24181d5d1f6d..9a93c13df8dde3 100644 --- a/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp +++ b/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp @@ -1345,18 +1345,15 @@ void Vectorizer::mergeEquivalenceClasses(EquivalenceClassMap &EQClasses) const { LLVM_DEBUG({ dbgs() << "LSV: mergeEquivalenceClasses: before merging:\n"; for (const auto &EC : EQClasses) { - dbgs() << " Key: ([" << std::get<0>(EC.first) - << "]: " << *std::get<0>(EC.first) << ", " << std::get<1>(EC.first) - << ", " << std::get<2>(EC.first) << ", " - << static_cast(std::get<3>(EC.first)) << ")\n"; + dbgs() << " Key: {" << EC.first << "}\n"; for (const auto &Inst : EC.second) - dbgs() << "\tInst: " << *Inst << '\n'; + dbgs() << " Inst: " << *Inst << '\n'; } }); LLVM_DEBUG({ dbgs() << "LSV: mergeEquivalenceClasses: RedKeyToUOMap:\n"; for (const auto &RedKeyToUO : RedKeyToUOMap) { - dbgs() << " Reduced key: (" << std::get<0>(RedKeyToUO.first) << ", " + dbgs() << " Reduced key: (" << std::get<0>(RedKeyToUO.first) << ", " << std::get<1>(RedKeyToUO.first) << ", " << static_cast(std::get<2>(RedKeyToUO.first)) << ") --> " << RedKeyToUO.second.size() << " underlying objects:\n"; @@ -1402,8 +1399,10 @@ void Vectorizer::mergeEquivalenceClasses(EquivalenceClassMap &EQClasses) const { std::get<2>(RedKey)}; EqClassKey KeyTo{UltimateTarget, std::get<0>(RedKey), std::get<1>(RedKey), std::get<2>(RedKey)}; - const auto &VecFrom = EQClasses[KeyFrom]; + // The entry for KeyFrom is guarantted to exist, unlike KeyTo. Thus, + // request the reference to the instructions vector for KeyTo first. const auto &VecTo = EQClasses[KeyTo]; + const auto &VecFrom = EQClasses[KeyFrom]; SmallVector MergedVec; std::merge(VecFrom.begin(), VecFrom.end(), VecTo.begin(), VecTo.end(), std::back_inserter(MergedVec), @@ -1417,12 +1416,9 @@ void Vectorizer::mergeEquivalenceClasses(EquivalenceClassMap &EQClasses) const { LLVM_DEBUG({ dbgs() << "LSV: mergeEquivalenceClasses: after merging:\n"; for (const auto &EC : EQClasses) { - dbgs() << " Key: ([" << std::get<0>(EC.first) - << "]: " << *std::get<0>(EC.first) << ", " << std::get<1>(EC.first) - << ", " << std::get<2>(EC.first) << ", " - << static_cast(std::get<3>(EC.first)) << ")\n"; + dbgs() << " Key: {" << EC.first << "}\n"; for (const auto &Inst : EC.second) - dbgs() << "\tInst: " << *Inst << '\n'; + dbgs() << " Inst: " << *Inst << '\n'; } }); } diff --git a/llvm/test/Transforms/LoadStoreVectorizer/X86/massive_indirection.ll b/llvm/test/Transforms/LoadStoreVectorizer/X86/massive_indirection.ll index c4b0d2e311d9d7..fe8a7e58a6a575 100644 --- a/llvm/test/Transforms/LoadStoreVectorizer/X86/massive_indirection.ll +++ b/llvm/test/Transforms/LoadStoreVectorizer/X86/massive_indirection.ll @@ -140,3 +140,41 @@ define void @v1_4_4_4_2_1_to_v8_8_levels_6_7(i32 %arg0, ptr addrspace(3) align 1 .exit_point: ret void } + +; The regression test for merging equivalence classes. It is reduced and adapted +; for LSV from llvm/test/CodeGen/NVPTX/variadics-backend.ll, which failed at +; post-commit checks with memory sanitizer on the initial attempt to implement +; the merging of the equivalence classes. +define void @variadics1(ptr %vlist) { +; CHECK-LABEL: define void @variadics1( +; CHECK-SAME: ptr [[VLIST:%.*]]) #[[ATTR0]] { +; CHECK-NEXT: [[ARGP_CUR7_ALIGNED2:%.*]] = call ptr @llvm.ptrmask.p0.i64(ptr [[VLIST]], i64 0) +; CHECK-NEXT: [[ARGP_NEXT8:%.*]] = getelementptr i8, ptr [[ARGP_CUR7_ALIGNED2]], i64 8 +; CHECK-NEXT: [[X0:%.*]] = getelementptr i8, ptr [[ARGP_NEXT8]], i32 7 +; CHECK-NEXT: [[ARGP_CUR11_ALIGNED:%.*]] = call ptr @llvm.ptrmask.p0.i64(ptr [[X0]], i64 0) +; CHECK-NEXT: [[ARGP_NEXT12:%.*]] = getelementptr i8, ptr [[ARGP_CUR11_ALIGNED]], i64 8 +; CHECK-NEXT: [[X2:%.*]] = getelementptr i8, ptr [[ARGP_NEXT12]], i32 7 +; CHECK-NEXT: [[ARGP_CUR16_ALIGNED:%.*]] = call ptr @llvm.ptrmask.p0.i64(ptr [[X2]], i64 0) +; CHECK-NEXT: [[TMP1:%.*]] = load <2 x double>, ptr [[ARGP_CUR16_ALIGNED]], align 4294967296 +; CHECK-NEXT: [[X31:%.*]] = extractelement <2 x double> [[TMP1]], i32 0 +; CHECK-NEXT: [[X42:%.*]] = extractelement <2 x double> [[TMP1]], i32 1 +; CHECK-NEXT: [[X5:%.*]] = fadd double [[X42]], [[X31]] +; CHECK-NEXT: store double [[X5]], ptr null, align 8 +; CHECK-NEXT: ret void +; + %argp.cur7.aligned2 = call ptr @llvm.ptrmask.p0.i64(ptr %vlist, i64 0) + %argp.next8 = getelementptr i8, ptr %argp.cur7.aligned2, i64 8 + %x0 = getelementptr i8, ptr %argp.next8, i32 7 + %argp.cur11.aligned = call ptr @llvm.ptrmask.p0.i64(ptr %x0, i64 0) + %argp.next12 = getelementptr i8, ptr %argp.cur11.aligned, i64 8 + %x2 = getelementptr i8, ptr %argp.next12, i32 7 + %argp.cur16.aligned = call ptr @llvm.ptrmask.p0.i64(ptr %x2, i64 0) + %x3 = load double, ptr %argp.cur16.aligned, align 8 + %argp.cur16.aligned_off8 = getelementptr i8, ptr %argp.cur16.aligned, i32 8 + %x4 = load double, ptr %argp.cur16.aligned_off8, align 8 + %x5 = fadd double %x4, %x3 + store double %x5, ptr null, align 8 + ret void +} + +declare ptr @llvm.ptrmask.p0.i64(ptr, i64)