From 066bd1841323a3a688f1b98f5a6dbcf10073e0d7 Mon Sep 17 00:00:00 2001 From: "Cai, Justin" Date: Thu, 31 Oct 2024 15:14:40 +0000 Subject: [PATCH 1/2] [SYCL] Add dummy image generation for virtual functions --- .../sycl-post-link/virtual-functions/dummy.ll | 37 +++++++++++++ llvm/tools/sycl-post-link/sycl-post-link.cpp | 54 +++++++++++++++++-- 2 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 llvm/test/tools/sycl-post-link/virtual-functions/dummy.ll diff --git a/llvm/test/tools/sycl-post-link/virtual-functions/dummy.ll b/llvm/test/tools/sycl-post-link/virtual-functions/dummy.ll new file mode 100644 index 0000000000000..8361ed22c3537 --- /dev/null +++ b/llvm/test/tools/sycl-post-link/virtual-functions/dummy.ll @@ -0,0 +1,37 @@ +; RUN: sycl-post-link -split=auto -properties -S < %s -o %t.table +; RUN: FileCheck %s --input-file=%t.table --check-prefix=CHECK-TABLE +; RUN: FileCheck %s --input-file=%t_0.ll --check-prefix=CHECK-FP64-SPLIT +; RUN: FileCheck %s --input-file=%t_1.ll --check-prefix=CHECK-FP64-DUMMY +; RUN: FileCheck %s --input-file=%t_1.prop --check-prefix=CHECK-FP64-DUMMY-PROPS +; RUN: FileCheck %s --input-file=%t_2.ll --check-prefix=CHECK-FP32-SPLIT + +; CHECK-TABLE: _0.prop +; CHECK-TABLE-NEXT: _1.prop +; CHECK-TABLE-NEXT: _2.prop + +; CHECK-FP64-SPLIT: define spir_func void @bar() +; CHECK-FP32-SPLIT: define spir_func void @foo() + +; CHECK-FP64-DUMMY: define spir_func void @bar() +; CHECK-FP64-DUMMY-NEXT: entry: +; CHECK-FP64-DUMMY-NEXT: ret void + +; CHECK-FP64-DUMMY-PROPS: dummy=1 + +define spir_func void @foo() #1 { + %x = alloca float + ret void +} + +define spir_func void @bar() #1 !sycl_used_aspects !1 { + %x = alloca double + %d = load double, ptr %x + %res = fadd double %d, %d + ret void +} + +attributes #1 = { "sycl-module-id"="v.cpp" "indirectly-callable"="setA" } + +!sycl_aspects = !{!0} +!0 = !{!"fp64", i32 6} +!1 = !{i32 6} diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index 3800c5875e44f..c68c5ebf46953 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -306,7 +306,8 @@ std::string saveModuleIR(Module &M, int I, StringRef Suff) { std::string saveModuleProperties(module_split::ModuleDesc &MD, const GlobalBinImageProps &GlobProps, int I, - StringRef Suff, StringRef Target = "") { + StringRef Suff, StringRef Target = "", + bool IsDummy = false) { auto PropSet = computeModuleProperties(MD.getModule(), MD.entries(), GlobProps); @@ -318,6 +319,10 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, NewSuff += Target; } + if (IsDummy) { + PropSet.add(PropSetRegTy::SYCL_VIRTUAL_FUNCTIONS, "dummy", 1); + } + std::error_code EC; std::string SCFile = makeResultFileName(".prop", I, NewSuff); raw_fd_ostream SCOut(SCFile, EC); @@ -416,7 +421,8 @@ void addTableRow(util::SimpleTable &Table, // IR component saving is skipped, and this file name is recorded as such in // the result. void saveModule(std::vector> &OutTables, - module_split::ModuleDesc &MD, int I, StringRef IRFilename) { + module_split::ModuleDesc &MD, int I, StringRef IRFilename, + bool IsDummy = false) { IrPropSymFilenameTriple BaseTriple; StringRef Suffix = getModuleSuffix(MD); MD.saveSplitInformationAsMetadata(); @@ -440,8 +446,8 @@ void saveModule(std::vector> &OutTables, GlobalBinImageProps Props = {EmitKernelParamInfo, EmitProgramMetadata, EmitExportedSymbols, EmitImportedSymbols, DeviceGlobals}; - CopyTriple.Prop = - saveModuleProperties(MD, Props, I, Suffix, OutputFile.Target); + CopyTriple.Prop = saveModuleProperties(MD, Props, I, Suffix, + OutputFile.Target, IsDummy); } addTableRow(*Table, CopyTriple); } @@ -741,6 +747,36 @@ bool isTargetCompatibleWithModule(const std::string &Target, return true; } +std::optional +makeDummy(module_split::ModuleDesc &MD) { + bool hasVirtualFunctions = false; + bool hasOptionalKernelFeatures = false; + for (Function &F : MD.getModule().functions()) { + if (F.hasFnAttribute("indirectly-callable")) + hasVirtualFunctions = true; + if (F.getMetadata("sycl_used_aspects")) + hasOptionalKernelFeatures = true; + if (hasVirtualFunctions && hasOptionalKernelFeatures) + break; + } + if (!hasVirtualFunctions || !hasOptionalKernelFeatures) + return {}; + + auto MDCopy = MD.clone(); + + for (Function &F : MDCopy.getModule().functions()) { + if (!F.hasFnAttribute("indirectly-callable")) + continue; + + F.erase(F.begin(), F.end()); + BasicBlock *newBB = BasicBlock::Create(F.getContext(), "entry", &F); + IRBuilder<> builder(newBB); + builder.CreateRetVoid(); + } + + return MDCopy; +} + std::vector> processInputModule(std::unique_ptr M) { // Construct the resulting table which will accumulate all the outputs. @@ -893,6 +929,16 @@ processInputModule(std::unique_ptr M) { ++ID; } + + bool dummyEmitted = false; + for (module_split::ModuleDesc &IrMD : MMs) { + if (auto Dummy = makeDummy(IrMD)) { + saveModule(Tables, *Dummy, ID, OutIRFileName, /*IsDummy*/ true); + dummyEmitted = true; + } + } + if (dummyEmitted) + ++ID; } return Tables; } From 94d63ff0656a1aba918ab0414296ba214cd3920f Mon Sep 17 00:00:00 2001 From: "Cai, Justin" Date: Thu, 5 Dec 2024 15:40:27 +0000 Subject: [PATCH 2/2] Ensure dummy image has same optional kernel features as the image it is created from --- .../include/llvm/SYCLLowerIR/ModuleSplitter.h | 4 + llvm/lib/SYCLLowerIR/ModuleSplitter.cpp | 9 +++ .../sycl-post-link/virtual-functions/dummy.ll | 2 +- llvm/tools/sycl-post-link/sycl-post-link.cpp | 81 +++++++++++-------- 4 files changed, 60 insertions(+), 36 deletions(-) diff --git a/llvm/include/llvm/SYCLLowerIR/ModuleSplitter.h b/llvm/include/llvm/SYCLLowerIR/ModuleSplitter.h index e622db50dd364..1f6ecb54a0cce 100644 --- a/llvm/include/llvm/SYCLLowerIR/ModuleSplitter.h +++ b/llvm/include/llvm/SYCLLowerIR/ModuleSplitter.h @@ -130,6 +130,7 @@ class ModuleDesc { EntryPointGroup EntryPoints; bool IsTopLevel = false; mutable std::optional Reqs; + bool IsDummyImage = false; public: struct Properties { @@ -225,6 +226,9 @@ class ModuleDesc { void saveSplitInformationAsMetadata(); + ModuleDesc makeDummy() const; + bool isDummyImage() { return IsDummyImage; } + #ifndef NDEBUG void verifyESIMDProperty() const; void dump() const; diff --git a/llvm/lib/SYCLLowerIR/ModuleSplitter.cpp b/llvm/lib/SYCLLowerIR/ModuleSplitter.cpp index 904424f93dae6..ade3955600564 100644 --- a/llvm/lib/SYCLLowerIR/ModuleSplitter.cpp +++ b/llvm/lib/SYCLLowerIR/ModuleSplitter.cpp @@ -816,6 +816,15 @@ void ModuleDesc::saveSplitInformationAsMetadata() { SpecConstantsPass::SPEC_CONST_DEFAULT_VAL_MODULE_MD_STRING); } +ModuleDesc ModuleDesc::makeDummy() const { + ModuleDesc MD(CloneModule(getModule())); + MD.EntryPoints = EntryPoints; + MD.IsTopLevel = IsTopLevel; + MD.Reqs = Reqs; + MD.IsDummyImage = true; + return MD; +} + void EntryPointGroup::saveNames(std::vector &Dest) const { Dest.reserve(Dest.size() + Functions.size()); std::transform(Functions.begin(), Functions.end(), diff --git a/llvm/test/tools/sycl-post-link/virtual-functions/dummy.ll b/llvm/test/tools/sycl-post-link/virtual-functions/dummy.ll index 8361ed22c3537..c2f7a3f81f85b 100644 --- a/llvm/test/tools/sycl-post-link/virtual-functions/dummy.ll +++ b/llvm/test/tools/sycl-post-link/virtual-functions/dummy.ll @@ -16,7 +16,7 @@ ; CHECK-FP64-DUMMY-NEXT: entry: ; CHECK-FP64-DUMMY-NEXT: ret void -; CHECK-FP64-DUMMY-PROPS: dummy=1 +; CHECK-FP64-DUMMY-PROPS: dummy-image=1 define spir_func void @foo() #1 { %x = alloca float diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index 53cef0a2276cc..0d012546a4fe7 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -59,6 +59,7 @@ #include "llvm/Transforms/Scalar/DCE.h" #include "llvm/Transforms/Scalar/EarlyCSE.h" #include "llvm/Transforms/Scalar/SROA.h" +#include "llvm/Transforms/Utils/Cloning.h" #include "llvm/Transforms/Utils/GlobalStatus.h" #include @@ -296,18 +297,41 @@ void saveModuleIR(Module &M, StringRef OutFilename) { MPM.run(M, MAM); } -std::string saveModuleIR(Module &M, int I, StringRef Suff) { - DUMP_ENTRY_POINTS(M, EmitOnlyKernelsAsEntryPoints, "saving IR"); +std::unique_ptr makeDummyImageIR(const Module &M) { + auto MCopy = CloneModule(M); + for (Function &F : MCopy->functions()) { + if (!F.hasFnAttribute("indirectly-callable")) + continue; + + F.erase(F.begin(), F.end()); + BasicBlock *newBB = BasicBlock::Create(F.getContext(), "entry", &F); + IRBuilder<> builder(newBB); + if (F.getReturnType()->isVoidTy()) + builder.CreateRetVoid(); + else + builder.CreateRet(UndefValue::get(F.getReturnType())); + } + return MCopy; +} + +std::string saveModuleIR(module_split::ModuleDesc &MD, int I, StringRef Suff) { + std::unique_ptr Storage; + Module *M = &MD.getModule(); + if (MD.isDummyImage()) { + Storage = makeDummyImageIR(MD.getModule()); + M = Storage.get(); + } + + DUMP_ENTRY_POINTS(*M, EmitOnlyKernelsAsEntryPoints, "saving IR"); StringRef FileExt = (OutputAssembly) ? ".ll" : ".bc"; std::string OutFilename = makeResultFileName(FileExt, I, Suff); - saveModuleIR(M, OutFilename); + saveModuleIR(*M, OutFilename); return OutFilename; } std::string saveModuleProperties(module_split::ModuleDesc &MD, const GlobalBinImageProps &GlobProps, int I, - StringRef Suff, StringRef Target = "", - bool IsDummy = false) { + StringRef Suff, StringRef Target = "") { auto PropSet = computeModuleProperties(MD.getModule(), MD.entries(), GlobProps); @@ -319,9 +343,8 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, NewSuff += Target; } - if (IsDummy) { - PropSet.add(PropSetRegTy::SYCL_VIRTUAL_FUNCTIONS, "dummy", 1); - } + if (MD.isDummyImage()) + PropSet.add(PropSetRegTy::SYCL_VIRTUAL_FUNCTIONS, "dummy-image", 1); std::error_code EC; std::string SCFile = makeResultFileName(".prop", I, NewSuff); @@ -421,8 +444,7 @@ void addTableRow(util::SimpleTable &Table, // IR component saving is skipped, and this file name is recorded as such in // the result. void saveModule(std::vector> &OutTables, - module_split::ModuleDesc &MD, int I, StringRef IRFilename, - bool IsDummy = false) { + module_split::ModuleDesc &MD, int I, StringRef IRFilename) { IrPropSymFilenameTriple BaseTriple; StringRef Suffix = getModuleSuffix(MD); MD.saveSplitInformationAsMetadata(); @@ -431,7 +453,7 @@ void saveModule(std::vector> &OutTables, BaseTriple.Ir = IRFilename.str(); } else { MD.cleanup(); - BaseTriple.Ir = saveModuleIR(MD.getModule(), I, Suffix); + BaseTriple.Ir = saveModuleIR(MD, I, Suffix); } if (DoSymGen) { // save the names of the entry points - the symbol table @@ -446,8 +468,8 @@ void saveModule(std::vector> &OutTables, GlobalBinImageProps Props = {EmitKernelParamInfo, EmitProgramMetadata, EmitExportedSymbols, EmitImportedSymbols, DeviceGlobals}; - CopyTriple.Prop = saveModuleProperties(MD, Props, I, Suffix, - OutputFile.Target, IsDummy); + CopyTriple.Prop = + saveModuleProperties(MD, Props, I, Suffix, OutputFile.Target); } addTableRow(*Table, CopyTriple); } @@ -747,11 +769,10 @@ bool isTargetCompatibleWithModule(const std::string &Target, return true; } -std::optional -makeDummy(module_split::ModuleDesc &MD) { +bool hasVirtualFunctionsAndOptionalKernelFeatures(const Module &M) { bool hasVirtualFunctions = false; bool hasOptionalKernelFeatures = false; - for (Function &F : MD.getModule().functions()) { + for (const Function &F : M.functions()) { if (F.hasFnAttribute("indirectly-callable")) hasVirtualFunctions = true; if (F.getMetadata("sycl_used_aspects")) @@ -759,22 +780,7 @@ makeDummy(module_split::ModuleDesc &MD) { if (hasVirtualFunctions && hasOptionalKernelFeatures) break; } - if (!hasVirtualFunctions || !hasOptionalKernelFeatures) - return {}; - - auto MDCopy = MD.clone(); - - for (Function &F : MDCopy.getModule().functions()) { - if (!F.hasFnAttribute("indirectly-callable")) - continue; - - F.erase(F.begin(), F.end()); - BasicBlock *newBB = BasicBlock::Create(F.getContext(), "entry", &F); - IRBuilder<> builder(newBB); - builder.CreateRetVoid(); - } - - return MDCopy; + return hasVirtualFunctions && hasOptionalKernelFeatures; } std::vector> @@ -932,11 +938,16 @@ processInputModule(std::unique_ptr M) { ++ID; } + // For kernels with virtual functions and optional kernel features, generate + // a dummy image to avoid link errors. A dummy image for a set of virtual + // functions is a module with the same set of virtual functions, but with + // those function bodies replaced with just a return. bool dummyEmitted = false; for (module_split::ModuleDesc &IrMD : MMs) { - if (auto Dummy = makeDummy(IrMD)) { - saveModule(Tables, *Dummy, ID, OutIRFileName, /*IsDummy*/ true); - dummyEmitted = true; + if ((dummyEmitted = hasVirtualFunctionsAndOptionalKernelFeatures( + IrMD.getModule()))) { + auto DummyImage = IrMD.makeDummy(); + saveModule(Tables, DummyImage, ID, OutIRFileName); } } if (dummyEmitted)