From 67a5e37dc7d78ae7803ff5e96f23d2980a292b42 Mon Sep 17 00:00:00 2001 From: Konstantin S Bobrovsky Date: Wed, 1 Apr 2020 22:19:46 -0700 Subject: [PATCH 1/3] [SYCL] Fix sycl-post-link when no split and symbols are requested. Signed-off-by: Konstantin S Bobrovsky --- .../tools/sycl-post-link/sym_but_no_split.ll | 25 ++++++++++++ llvm/tools/sycl-post-link/sycl-post-link.cpp | 40 +++++++++++++++---- 2 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 llvm/test/tools/sycl-post-link/sym_but_no_split.ll diff --git a/llvm/test/tools/sycl-post-link/sym_but_no_split.ll b/llvm/test/tools/sycl-post-link/sym_but_no_split.ll new file mode 100644 index 000000000000..12ca6b23c774 --- /dev/null +++ b/llvm/test/tools/sycl-post-link/sym_but_no_split.ll @@ -0,0 +1,25 @@ +; This test checks that the post-link tool produces a correct resulting file +; table and a symbol file for an input module with two kernels when no code +; splitting is requested. +; +; RUN: sycl-post-link -symbols -spec-const=rt -S %s -o %T/files.table +; RUN: FileCheck %s -input-file=%T/files.table --check-prefixes CHECK-TABLE +; RUN: FileCheck %s -input-file=%T/files_0.sym --match-full-lines --check-prefixes CHECK-SYM + +define dso_local spir_kernel void @KERNEL_AAA() { +; CHECK-SYM-NOT: {{[a-zA-Z0-9._@]+}} +; CHECK-SYM: KERNEL_AAA +entry: + ret void +} + +define dso_local spir_kernel void @KERNEL_BBB() { +; CHECK-SYM-NEXT: KERNEL_BBB +; CHECK-SYM-EMPTY: +entry: + ret void +} + +; CHECK-TABLE: [Code|Properties|Symbols] +; CHECK-TABLE-NEXT: {{.*}}files_0.sym +; CHECK-TABLE-EMPTY: diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index 39d21018f764..5b61dcf682f3 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -121,23 +121,45 @@ static void writeToFile(std::string Filename, std::string Content) { OS.close(); } +// Describes scope covered by each entry in the module-kernel map populated by +// the function below. +enum KernelMapEntryScope { + Scope_PerKernel, // one entry per kernel + Scope_PerModule, // one entry per module + Scope_Global // single entry in the map for all kernels +}; + // Output parameter ResKernelModuleMap is a map containing groups of kernels // with same values of the sycl-module-id attribute. // The function fills ResKernelModuleMap using input module M. static void collectKernelModuleMap( Module &M, std::map> &ResKernelModuleMap, - bool OneKernelPerModule) { - - constexpr char ATTR_SYCL_MODULE_ID[] = "sycl-module-id"; + KernelMapEntryScope EntryScope) { for (auto &F : M.functions()) { if (F.getCallingConv() == CallingConv::SPIR_KERNEL) { - if (OneKernelPerModule) { + switch (EntryScope) { + case Scope_PerKernel: ResKernelModuleMap[F.getName()].push_back(&F); - } else if (F.hasFnAttribute(ATTR_SYCL_MODULE_ID)) { + break; + case Scope_PerModule: { + constexpr char ATTR_SYCL_MODULE_ID[] = "sycl-module-id"; + + if (!F.hasFnAttribute(ATTR_SYCL_MODULE_ID)) + error("no '" + Twine(ATTR_SYCL_MODULE_ID) + + "' attribute in kernel '" + F.getName() + + "', per-module split not possible"); Attribute Id = F.getFnAttribute(ATTR_SYCL_MODULE_ID); StringRef Val = Id.getValueAsString(); ResKernelModuleMap[Val].push_back(&F); + break; + } + case Scope_Global: + // the map key is not significant here + ResKernelModuleMap[""].push_back(&F); + break; + default: + llvm_unreachable("unknown scope"); } } } @@ -375,8 +397,12 @@ int main(int argc, char **argv) { std::map> GlobalsSet; - if (DoSplit || DoSymGen) - collectKernelModuleMap(*MPtr, GlobalsSet, SplitMode == SPLIT_PER_KERNEL); + if (DoSplit || DoSymGen) { + KernelMapEntryScope Scope = Scope_Global; + if (DoSplit) + Scope = SplitMode == SPLIT_PER_KERNEL ? Scope_PerKernel : Scope_PerModule; + collectKernelModuleMap(*MPtr, GlobalsSet, Scope); + } std::vector> ResultModules; std::vector ResultSpecIDMaps; From 27b88f52ba9f3bd746fce8a9e3c71db7ca57d273 Mon Sep 17 00:00:00 2001 From: Konstantin S Bobrovsky Date: Thu, 2 Apr 2020 15:50:34 -0700 Subject: [PATCH 2/3] [SYCL] Implement review comments for PR #1454. Signed-off-by: Konstantin S Bobrovsky --- llvm/test/tools/sycl-post-link/sym_but_no_split.ll | 6 +++--- llvm/tools/sycl-post-link/sycl-post-link.cpp | 14 ++++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/llvm/test/tools/sycl-post-link/sym_but_no_split.ll b/llvm/test/tools/sycl-post-link/sym_but_no_split.ll index 12ca6b23c774..ec2050ad8775 100644 --- a/llvm/test/tools/sycl-post-link/sym_but_no_split.ll +++ b/llvm/test/tools/sycl-post-link/sym_but_no_split.ll @@ -2,9 +2,9 @@ ; table and a symbol file for an input module with two kernels when no code ; splitting is requested. ; -; RUN: sycl-post-link -symbols -spec-const=rt -S %s -o %T/files.table -; RUN: FileCheck %s -input-file=%T/files.table --check-prefixes CHECK-TABLE -; RUN: FileCheck %s -input-file=%T/files_0.sym --match-full-lines --check-prefixes CHECK-SYM +; RUN: sycl-post-link -symbols -spec-const=rt -S %s -o %t.files.table +; RUN: FileCheck %s -input-file=%t.files.table --check-prefixes CHECK-TABLE +; RUN: FileCheck %s -input-file=%t.files_0.sym --match-full-lines --check-prefixes CHECK-SYM define dso_local spir_kernel void @KERNEL_AAA() { ; CHECK-SYM-NOT: {{[a-zA-Z0-9._@]+}} diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index 5b61dcf682f3..1507c181deec 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -122,16 +122,19 @@ static void writeToFile(std::string Filename, std::string Content) { } // Describes scope covered by each entry in the module-kernel map populated by -// the function below. +// the collectKernelModuleMap function. enum KernelMapEntryScope { Scope_PerKernel, // one entry per kernel Scope_PerModule, // one entry per module Scope_Global // single entry in the map for all kernels }; -// Output parameter ResKernelModuleMap is a map containing groups of kernels -// with same values of the sycl-module-id attribute. -// The function fills ResKernelModuleMap using input module M. +// This function decides how kernels of the input module M will be distributed +// ("split") into multiple modules based on the command options and IR +// attributes. The desision is recorded in the output map parameter +// ResKernelModuleMap which maps some key to a group of kernels. Each such group +// along with IR it depends on (globals, functions from its call graph,...) will +// constitute a separate module. static void collectKernelModuleMap( Module &M, std::map> &ResKernelModuleMap, KernelMapEntryScope EntryScope) { @@ -145,6 +148,9 @@ static void collectKernelModuleMap( case Scope_PerModule: { constexpr char ATTR_SYCL_MODULE_ID[] = "sycl-module-id"; + // TODO It may make sense to group all kernels w/o the attribute into + // a separate module rather than issuing an error. Should probably be + // controlled by an option. if (!F.hasFnAttribute(ATTR_SYCL_MODULE_ID)) error("no '" + Twine(ATTR_SYCL_MODULE_ID) + "' attribute in kernel '" + F.getName() + From 6f76fd8c402e61dc93f7690ea7d5d830bd2b7640 Mon Sep 17 00:00:00 2001 From: kbobrovs Date: Fri, 3 Apr 2020 08:10:34 -0700 Subject: [PATCH 3/3] Update llvm/tools/sycl-post-link/sycl-post-link.cpp Co-Authored-By: Mariya Podchishchaeva --- llvm/tools/sycl-post-link/sycl-post-link.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index 1507c181deec..2d1d01ce7801 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -131,7 +131,7 @@ enum KernelMapEntryScope { // This function decides how kernels of the input module M will be distributed // ("split") into multiple modules based on the command options and IR -// attributes. The desision is recorded in the output map parameter +// attributes. The decision is recorded in the output map parameter // ResKernelModuleMap which maps some key to a group of kernels. Each such group // along with IR it depends on (globals, functions from its call graph,...) will // constitute a separate module.