From a5edd23955572b698f9247ceb05d39d00546286c Mon Sep 17 00:00:00 2001 From: Jakub Chlanda Date: Tue, 8 Aug 2023 09:49:27 +0100 Subject: [PATCH 1/8] [SYCL] Update kernel function metadata after local accessor pass --- llvm/include/llvm/SYCLLowerIR/TargetHelpers.h | 1 + .../LocalAccessorToSharedMemory.cpp | 9 ++++- llvm/lib/SYCLLowerIR/TargetHelpers.cpp | 38 ++++++++++++++++++- ...r-to-shared-memory-basic-transformation.ll | 12 ++++-- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/llvm/include/llvm/SYCLLowerIR/TargetHelpers.h b/llvm/include/llvm/SYCLLowerIR/TargetHelpers.h index b2b383237a705..fba50396e6be2 100644 --- a/llvm/include/llvm/SYCLLowerIR/TargetHelpers.h +++ b/llvm/include/llvm/SYCLLowerIR/TargetHelpers.h @@ -28,6 +28,7 @@ struct KernelPayload { KernelPayload(Function *Kernel, MDNode *MD = nullptr); Function *Kernel; MDNode *MD; + SmallVector DependentMDs; }; ArchType getArchType(const Module &M); diff --git a/llvm/lib/SYCLLowerIR/LocalAccessorToSharedMemory.cpp b/llvm/lib/SYCLLowerIR/LocalAccessorToSharedMemory.cpp index 18c5bb00fb488..3460ac8e1dc5a 100644 --- a/llvm/lib/SYCLLowerIR/LocalAccessorToSharedMemory.cpp +++ b/llvm/lib/SYCLLowerIR/LocalAccessorToSharedMemory.cpp @@ -208,7 +208,12 @@ Function *LocalAccessorToSharedMemoryPass::processKernel(Module &M, void LocalAccessorToSharedMemoryPass::postProcessKernels( SmallVectorImpl> &NewToOldKernels) { for (auto &Pair : NewToOldKernels) { - std::get<1>(Pair).MD->replaceOperandWith( - 0, llvm::ConstantAsMetadata::get(std::get<0>(Pair))); + auto KP = std::get<1>(Pair); + auto *F = std::get<0>(Pair); + KP.MD->replaceOperandWith(0, llvm::ConstantAsMetadata::get(F)); + // The MD node of the kernel has been altered, make sure that all the + // dependent nodes are kept up to date. + for (auto *D : KP.DependentMDs) + D->replaceOperandWith(0, llvm::ConstantAsMetadata::get(F)); } } diff --git a/llvm/lib/SYCLLowerIR/TargetHelpers.cpp b/llvm/lib/SYCLLowerIR/TargetHelpers.cpp index a4a7e35cfc297..c8c4014f3a353 100644 --- a/llvm/lib/SYCLLowerIR/TargetHelpers.cpp +++ b/llvm/lib/SYCLLowerIR/TargetHelpers.cpp @@ -56,6 +56,7 @@ void populateKernels(Module &M, SmallVectorImpl &Kernels, if (!AnnotationMetadata) return; + std::vector PossibleDependencies; // It is possible that the annotations node contains multiple pointers to the // same metadata, recognise visited ones. SmallSet Visited; @@ -70,9 +71,12 @@ void populateKernels(Module &M, SmallVectorImpl &Kernels, auto *Type = dyn_cast(MetadataNode->getOperand(1)); if (!Type) continue; - // Only process kernel entry points. - if (Type->getString() != "kernel") + // Only process kernel entry points, + if (Type->getString() != "kernel") { + // but keep track of other nodes that point to the same function. + PossibleDependencies.push_back(MetadataNode); continue; + } // Get a pointer to the entry point function from the metadata. const MDOperand &FuncOperand = MetadataNode->getOperand(0); @@ -82,6 +86,36 @@ void populateKernels(Module &M, SmallVectorImpl &Kernels, if (auto *Func = dyn_cast(FuncConstant->getValue())) Kernels.push_back(KernelPayload(Func, MetadataNode)); } + + // We need to match non-kernel metadata nodes using the kernel name to the + // kernel nodes. + for (auto &KP : Kernels) { + auto *KernelConstant = dyn_cast(KP.MD->getOperand(0)); + assert(KernelConstant); + auto KernelName = + dyn_cast(KernelConstant->getValue())->getFunction().getName(); + // Keep track of matched nodes, to decrease the search space with each + // iteration. + SmallVector ToDelete; + for (unsigned i = 0; i < PossibleDependencies.size(); ++i) { + auto *Dep = PossibleDependencies[i]; + const MDOperand &FuncOperand = Dep->getOperand(0); + if (!FuncOperand) + continue; + if (auto *FuncConstant = dyn_cast(FuncOperand)) + if (auto *Func = dyn_cast(FuncConstant->getValue())) + // We've found a match, append the dependent node to the kernel + // payload. + if (KernelName == Func->getFunction().getName()) { + KP.DependentMDs.push_back(Dep); + ToDelete.push_back(i); + } + + // Remove matched dependencies. + for (auto i = ToDelete.rbegin(); i != ToDelete.rend(); ++i) + PossibleDependencies.erase(PossibleDependencies.begin() + *i); + } + } } } // namespace TargetHelpers diff --git a/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll b/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll index b7e0103f5949d..fb8a66edfa3e9 100644 --- a/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll +++ b/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll @@ -4,7 +4,9 @@ source_filename = "basic-transformation.ll" target datalayout = "e-i64:64-i128:128-v16:16-v32:32-n16:32:64" target triple = "nvptx64-nvidia-cuda" -; This test checks that the transformation is applied in the basic case. +; This test checks that the transformation is applied in the basic case. It +; aslo makes sure that a non-kernel node using the function's signature gets +; correcly updated (`maxntid`). ; CHECK: @_ZTS14example_kernel_shared_mem = external addrspace(3) global [0 x i8], align 4 @@ -23,8 +25,8 @@ entry: ret void } -!nvvm.annotations = !{!0, !1, !2, !1, !3, !3, !3, !3, !4, !4, !3} -!nvvmir.version = !{!5} +!nvvm.annotations = !{!0, !1, !2, !1, !3, !3, !3, !3, !4, !4, !3, !5} +!nvvmir.version = !{!6} !0 = distinct !{void (i32 addrspace(3)*, i32 addrspace(1)*, i32)* @_ZTS14example_kernel, !"kernel", i32 1} ; CHECK: !0 = distinct !{void (i32, i32 addrspace(1)*, i32)* @_ZTS14example_kernel, !"kernel", i32 1} @@ -32,4 +34,6 @@ entry: !2 = !{null, !"align", i32 8, !"align", i32 65544, !"align", i32 131080} !3 = !{null, !"align", i32 16} !4 = !{null, !"align", i32 16, !"align", i32 65552, !"align", i32 131088} -!5 = !{i32 1, i32 4} +; CHECK: !5 = distinct !{void (i32, i32 addrspace(1)*, i32)* @_ZTS14example_kernel, !"maxntid", i32 256} +!5 = !{void (i32 addrspace(3)*, i32 addrspace(1)*, i32)* @_ZTS14example_kernel, !"maxntidx", i32 256} +!6 = !{i32 1, i32 4} From 18630ce1b26f33412c78cea6b8e2ae60f6172576 Mon Sep 17 00:00:00 2001 From: Jakub Chlanda Date: Tue, 8 Aug 2023 12:26:49 +0200 Subject: [PATCH 2/8] Update llvm/lib/SYCLLowerIR/LocalAccessorToSharedMemory.cpp Co-authored-by: Alexey Sachkov --- llvm/lib/SYCLLowerIR/LocalAccessorToSharedMemory.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/SYCLLowerIR/LocalAccessorToSharedMemory.cpp b/llvm/lib/SYCLLowerIR/LocalAccessorToSharedMemory.cpp index 3460ac8e1dc5a..febf46177a6c4 100644 --- a/llvm/lib/SYCLLowerIR/LocalAccessorToSharedMemory.cpp +++ b/llvm/lib/SYCLLowerIR/LocalAccessorToSharedMemory.cpp @@ -213,7 +213,7 @@ void LocalAccessorToSharedMemoryPass::postProcessKernels( KP.MD->replaceOperandWith(0, llvm::ConstantAsMetadata::get(F)); // The MD node of the kernel has been altered, make sure that all the // dependent nodes are kept up to date. - for (auto *D : KP.DependentMDs) + for (MDNode *D : KP.DependentMDs) D->replaceOperandWith(0, llvm::ConstantAsMetadata::get(F)); } } From 0b0ff4a7a7a13dcc03e970a995ef2c5a6d8bb753 Mon Sep 17 00:00:00 2001 From: Jakub Chlanda Date: Tue, 8 Aug 2023 12:27:14 +0200 Subject: [PATCH 3/8] Update llvm/lib/SYCLLowerIR/TargetHelpers.cpp Co-authored-by: Alexey Sachkov --- llvm/lib/SYCLLowerIR/TargetHelpers.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/lib/SYCLLowerIR/TargetHelpers.cpp b/llvm/lib/SYCLLowerIR/TargetHelpers.cpp index c8c4014f3a353..79801c3cada51 100644 --- a/llvm/lib/SYCLLowerIR/TargetHelpers.cpp +++ b/llvm/lib/SYCLLowerIR/TargetHelpers.cpp @@ -90,8 +90,7 @@ void populateKernels(Module &M, SmallVectorImpl &Kernels, // We need to match non-kernel metadata nodes using the kernel name to the // kernel nodes. for (auto &KP : Kernels) { - auto *KernelConstant = dyn_cast(KP.MD->getOperand(0)); - assert(KernelConstant); + auto *KernelConstant = cast(KP.MD->getOperand(0)); auto KernelName = dyn_cast(KernelConstant->getValue())->getFunction().getName(); // Keep track of matched nodes, to decrease the search space with each From e16850400fb30d561e9b4a193e53026de31353f6 Mon Sep 17 00:00:00 2001 From: Jakub Chlanda Date: Tue, 8 Aug 2023 12:27:38 +0200 Subject: [PATCH 4/8] Update llvm/lib/SYCLLowerIR/TargetHelpers.cpp Co-authored-by: Alexey Sachkov --- llvm/lib/SYCLLowerIR/TargetHelpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/SYCLLowerIR/TargetHelpers.cpp b/llvm/lib/SYCLLowerIR/TargetHelpers.cpp index 79801c3cada51..e2f3cfde0ac19 100644 --- a/llvm/lib/SYCLLowerIR/TargetHelpers.cpp +++ b/llvm/lib/SYCLLowerIR/TargetHelpers.cpp @@ -92,7 +92,7 @@ void populateKernels(Module &M, SmallVectorImpl &Kernels, for (auto &KP : Kernels) { auto *KernelConstant = cast(KP.MD->getOperand(0)); auto KernelName = - dyn_cast(KernelConstant->getValue())->getFunction().getName(); + cast(KernelConstant->getValue())->getFunction().getName(); // Keep track of matched nodes, to decrease the search space with each // iteration. SmallVector ToDelete; From d6fd04152937a1545c0f72d9040f5418c8558d76 Mon Sep 17 00:00:00 2001 From: Jakub Chlanda Date: Tue, 8 Aug 2023 11:37:49 +0100 Subject: [PATCH 5/8] replace ToDelete with HandledNodes set --- llvm/lib/SYCLLowerIR/TargetHelpers.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/llvm/lib/SYCLLowerIR/TargetHelpers.cpp b/llvm/lib/SYCLLowerIR/TargetHelpers.cpp index e2f3cfde0ac19..a40e6535799e6 100644 --- a/llvm/lib/SYCLLowerIR/TargetHelpers.cpp +++ b/llvm/lib/SYCLLowerIR/TargetHelpers.cpp @@ -88,31 +88,28 @@ void populateKernels(Module &M, SmallVectorImpl &Kernels, } // We need to match non-kernel metadata nodes using the kernel name to the - // kernel nodes. + // kernel nodes. To avoid checking matched nodes multiple times keep track of + // handled entries. + SmallSet HandledNodes; for (auto &KP : Kernels) { auto *KernelConstant = cast(KP.MD->getOperand(0)); auto KernelName = cast(KernelConstant->getValue())->getFunction().getName(); - // Keep track of matched nodes, to decrease the search space with each - // iteration. - SmallVector ToDelete; - for (unsigned i = 0; i < PossibleDependencies.size(); ++i) { - auto *Dep = PossibleDependencies[i]; + for (unsigned I = 0; I < PossibleDependencies.size(); ++I) { + if (HandledNodes.contains(I)) + continue; + MDNode *Dep = PossibleDependencies[I]; const MDOperand &FuncOperand = Dep->getOperand(0); if (!FuncOperand) continue; if (auto *FuncConstant = dyn_cast(FuncOperand)) if (auto *Func = dyn_cast(FuncConstant->getValue())) // We've found a match, append the dependent node to the kernel - // payload. + // payload and keep track of matched entries. if (KernelName == Func->getFunction().getName()) { KP.DependentMDs.push_back(Dep); - ToDelete.push_back(i); + HandledNodes.insert(I); } - - // Remove matched dependencies. - for (auto i = ToDelete.rbegin(); i != ToDelete.rend(); ++i) - PossibleDependencies.erase(PossibleDependencies.begin() + *i); } } } From 9c91d4ff30191c9ee9587690a55590100c0ac67e Mon Sep 17 00:00:00 2001 From: Jakub Chlanda Date: Wed, 9 Aug 2023 07:20:44 +0100 Subject: [PATCH 6/8] typo in a test --- .../local-accessor-to-shared-memory-basic-transformation.ll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll b/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll index fb8a66edfa3e9..910429e505f01 100644 --- a/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll +++ b/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll @@ -34,6 +34,6 @@ entry: !2 = !{null, !"align", i32 8, !"align", i32 65544, !"align", i32 131080} !3 = !{null, !"align", i32 16} !4 = !{null, !"align", i32 16, !"align", i32 65552, !"align", i32 131088} -; CHECK: !5 = distinct !{void (i32, i32 addrspace(1)*, i32)* @_ZTS14example_kernel, !"maxntid", i32 256} +; CHECK: !5 = distinct !{void (i32, i32 addrspace(1)*, i32)* @_ZTS14example_kernel, !"maxntidx", i32 256} !5 = !{void (i32 addrspace(3)*, i32 addrspace(1)*, i32)* @_ZTS14example_kernel, !"maxntidx", i32 256} !6 = !{i32 1, i32 4} From 21bee08362d035bcbfd2d8513ed4897ddc6b9c7c Mon Sep 17 00:00:00 2001 From: Jakub Chlanda Date: Wed, 16 Aug 2023 09:25:55 +0100 Subject: [PATCH 7/8] typo --- .../local-accessor-to-shared-memory-basic-transformation.ll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll b/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll index 910429e505f01..31785f3303a49 100644 --- a/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll +++ b/llvm/test/CodeGen/NVPTX/local-accessor-to-shared-memory-basic-transformation.ll @@ -5,7 +5,7 @@ target datalayout = "e-i64:64-i128:128-v16:16-v32:32-n16:32:64" target triple = "nvptx64-nvidia-cuda" ; This test checks that the transformation is applied in the basic case. It -; aslo makes sure that a non-kernel node using the function's signature gets +; also makes sure that a non-kernel node using the function's signature gets ; correcly updated (`maxntid`). ; CHECK: @_ZTS14example_kernel_shared_mem = external addrspace(3) global [0 x i8], align 4 From b60673d7e9cf57fcbf97ac34243fbf36d2609caa Mon Sep 17 00:00:00 2001 From: Jakub Chlanda Date: Wed, 16 Aug 2023 10:40:36 +0100 Subject: [PATCH 8/8] SmallVector > vector --- llvm/lib/SYCLLowerIR/TargetHelpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/SYCLLowerIR/TargetHelpers.cpp b/llvm/lib/SYCLLowerIR/TargetHelpers.cpp index a40e6535799e6..8c45148d181e4 100644 --- a/llvm/lib/SYCLLowerIR/TargetHelpers.cpp +++ b/llvm/lib/SYCLLowerIR/TargetHelpers.cpp @@ -56,7 +56,7 @@ void populateKernels(Module &M, SmallVectorImpl &Kernels, if (!AnnotationMetadata) return; - std::vector PossibleDependencies; + SmallVector PossibleDependencies; // It is possible that the annotations node contains multiple pointers to the // same metadata, recognise visited ones. SmallSet Visited;