-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[offload][SYCL] Add SYCL Module splitting #119713
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.
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
Conversation
This patch adds SYCL Module splitting - the necessary step in the SYCL compilation pipeline. Only 2 splitting modes are being added: by kernel and by source.
@llvm/pr-subscribers-llvm-transforms Author: Maksim Sabianin (maksimsab) ChangesThis patch adds SYCL Module splitting - the necessary step in the SYCL compilation pipeline. Only 2 splitting modes are being added in this patch: by kernel and by source. Patch is 56.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119713.diff 15 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/SYCLModuleSplit.h b/llvm/include/llvm/Transforms/Utils/SYCLModuleSplit.h
new file mode 100644
index 00000000000000..4df3e0321e9cda
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Utils/SYCLModuleSplit.h
@@ -0,0 +1,71 @@
+//===-------- SYCLModuleSplit.h - module split ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// Functionality to split a module into callgraphs. A callgraph here is a set
+// of entry points with all functions reachable from them via a call. The result
+// of the split is new modules containing corresponding callgraph.
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_SYCL_MODULE_SPLIT_H
+#define LLVM_SYCL_MODULE_SPLIT_H
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+#include <memory>
+#include <optional>
+#include <string>
+
+namespace llvm {
+
+class Module;
+
+enum class IRSplitMode {
+ IRSM_PER_TU, // one module per translation unit
+ IRSM_PER_KERNEL, // one module per kernel
+ IRSM_NONE // no splitting
+};
+
+/// \returns IRSplitMode value if \p S is recognized. Otherwise, std::nullopt is
+/// returned.
+std::optional<IRSplitMode> convertStringToSplitMode(StringRef S);
+
+/// The structure represents a split LLVM Module accompanied by additional
+/// information. Split Modules are being stored at disk due to the high RAM
+/// consumption during the whole splitting process.
+struct SYCLSplitModule {
+ std::string ModuleFilePath;
+ std::string Symbols;
+
+ SYCLSplitModule() = default;
+ SYCLSplitModule(const SYCLSplitModule &) = default;
+ SYCLSplitModule &operator=(const SYCLSplitModule &) = default;
+ SYCLSplitModule(SYCLSplitModule &&) = default;
+ SYCLSplitModule &operator=(SYCLSplitModule &&) = default;
+
+ SYCLSplitModule(std::string_view File, std::string Symbols)
+ : ModuleFilePath(File), Symbols(std::move(Symbols)) {}
+};
+
+struct ModuleSplitterSettings {
+ IRSplitMode Mode;
+ bool OutputAssembly = false; // Bitcode or LLVM IR.
+ StringRef OutputPrefix;
+};
+
+/// Parses the string table.
+Expected<SmallVector<SYCLSplitModule, 0>>
+parseSYCLSplitModulesFromFile(StringRef File);
+
+/// Splits the given module \p M according to the given \p Settings.
+Expected<SmallVector<SYCLSplitModule, 0>>
+splitSYCLModule(std::unique_ptr<Module> M, ModuleSplitterSettings Settings);
+
+} // namespace llvm
+
+#endif // LLVM_SYCL_MODULE_SPLIT_H
diff --git a/llvm/include/llvm/Transforms/Utils/SYCLUtils.h b/llvm/include/llvm/Transforms/Utils/SYCLUtils.h
new file mode 100644
index 00000000000000..53dec1139cd8ef
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Utils/SYCLUtils.h
@@ -0,0 +1,26 @@
+//===------------ SYCLUtils.h - SYCL utility functions --------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// Utility functions for SYCL.
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_TRANSFORMS_UTILS_SYCLUTILS_H
+#define LLVM_TRANSFORMS_UTILS_SYCLUTILS_H
+
+#include <string>
+#include <vector>
+
+namespace llvm {
+
+class raw_ostream;
+
+using SYCLStringTable = std::vector<std::vector<std::string>>;
+
+void writeSYCLStringTable(const SYCLStringTable &Table, raw_ostream &OS);
+
+} // namespace llvm
+
+#endif // LLVM_TRANSFORMS_UTILS_SYCLUTILS_H
diff --git a/llvm/lib/Transforms/Utils/CMakeLists.txt b/llvm/lib/Transforms/Utils/CMakeLists.txt
index 65bd3080662c4d..530cba5275dcb0 100644
--- a/llvm/lib/Transforms/Utils/CMakeLists.txt
+++ b/llvm/lib/Transforms/Utils/CMakeLists.txt
@@ -82,6 +82,8 @@ add_llvm_component_library(LLVMTransformUtils
SizeOpts.cpp
SplitModule.cpp
StripNonLineTableDebugInfo.cpp
+ SYCLModuleSplit.cpp
+ SYCLUtils.cpp
SymbolRewriter.cpp
UnifyFunctionExitNodes.cpp
UnifyLoopExits.cpp
diff --git a/llvm/lib/Transforms/Utils/SYCLModuleSplit.cpp b/llvm/lib/Transforms/Utils/SYCLModuleSplit.cpp
new file mode 100644
index 00000000000000..e6a36a1fba9695
--- /dev/null
+++ b/llvm/lib/Transforms/Utils/SYCLModuleSplit.cpp
@@ -0,0 +1,513 @@
+//===-------- SYCLModuleSplitter.cpp - split a module into callgraphs -----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// See comments in the header.
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/Utils/SYCLModuleSplit.h"
+#include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Bitcode/BitcodeWriterPass.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/InstIterator.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/PassManager.h"
+#include "llvm/IR/PassManagerImpl.h"
+#include "llvm/IRPrinter/IRPrintingPasses.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Transforms/IPO/GlobalDCE.h"
+#include "llvm/Transforms/IPO/StripDeadPrototypes.h"
+#include "llvm/Transforms/IPO/StripSymbols.h"
+#include "llvm/Transforms/Utils/Cloning.h"
+#include "llvm/Transforms/Utils/SYCLUtils.h"
+
+#include <map>
+#include <utility>
+
+using namespace llvm;
+
+#define DEBUG_TYPE "sycl_module_split"
+
+static bool isKernel(const Function &F) {
+ return F.getCallingConv() == CallingConv::SPIR_KERNEL ||
+ F.getCallingConv() == CallingConv::AMDGPU_KERNEL;
+}
+
+static bool isEntryPoint(const Function &F) {
+ // Skip declarations, if any: they should not be included into a vector of
+ // entry points groups or otherwise we will end up with incorrectly generated
+ // list of symbols.
+ if (F.isDeclaration())
+ return false;
+
+ // Kernels are always considered to be entry points
+ return isKernel(F);
+}
+
+namespace {
+
+// A vector that contains all entry point functions in a split module.
+using EntryPointSet = SetVector<const Function *>;
+
+/// Represents a named group entry points.
+struct EntryPointGroup {
+ std::string GroupName;
+ EntryPointSet Functions;
+
+ EntryPointGroup() = default;
+ EntryPointGroup(const EntryPointGroup &) = default;
+ EntryPointGroup &operator=(const EntryPointGroup &) = default;
+ EntryPointGroup(EntryPointGroup &&) = default;
+ EntryPointGroup &operator=(EntryPointGroup &&) = default;
+
+ EntryPointGroup(StringRef GroupName,
+ EntryPointSet Functions = EntryPointSet())
+ : GroupName(GroupName), Functions(std::move(Functions)) {}
+
+ void dump() const {
+ constexpr size_t INDENT = 4;
+ dbgs().indent(INDENT) << "ENTRY POINTS"
+ << " " << GroupName << " {\n";
+ for (const Function *F : Functions)
+ dbgs().indent(INDENT) << " " << F->getName() << "\n";
+
+ dbgs().indent(INDENT) << "}\n";
+ }
+};
+
+/// Annotates an llvm::Module with information necessary to perform and track
+/// the result of device code (llvm::Module instances) splitting:
+/// - entry points group from the module.
+class ModuleDesc {
+ std::unique_ptr<Module> M;
+ EntryPointGroup EntryPoints;
+
+public:
+ ModuleDesc() = delete;
+ ModuleDesc(const ModuleDesc &) = delete;
+ ModuleDesc &operator=(const ModuleDesc &) = delete;
+ ModuleDesc(ModuleDesc &&) = default;
+ ModuleDesc &operator=(ModuleDesc &&) = default;
+
+ ModuleDesc(std::unique_ptr<Module> M,
+ EntryPointGroup EntryPoints = EntryPointGroup())
+ : M(std::move(M)), EntryPoints(std::move(EntryPoints)) {
+ assert(this->M && "Module should be non-empty");
+ }
+
+ const EntryPointSet &entries() const { return EntryPoints.Functions; }
+ const EntryPointGroup &getEntryPointGroup() const { return EntryPoints; }
+ EntryPointSet &entries() { return EntryPoints.Functions; }
+ Module &getModule() { return *M; }
+ const Module &getModule() const { return *M; }
+
+ // Cleans up module IR - removes dead globals, debug info etc.
+ void cleanup() {
+ ModuleAnalysisManager MAM;
+ MAM.registerPass([&] { return PassInstrumentationAnalysis(); });
+ ModulePassManager MPM;
+ MPM.addPass(GlobalDCEPass()); // Delete unreachable globals.
+ MPM.addPass(StripDeadDebugInfoPass()); // Remove dead debug info.
+ MPM.addPass(StripDeadPrototypesPass()); // Remove dead func decls.
+ MPM.run(*M, MAM);
+ }
+
+ std::string makeSymbolTable() const {
+ SmallString<128> ST;
+ for (const Function *F : EntryPoints.Functions) {
+ ST += F->getName();
+ ST += "\n";
+ }
+
+ return std::string(ST);
+ }
+
+ void dump() const {
+ dbgs() << "ModuleDesc[" << M->getName() << "] {\n";
+ EntryPoints.dump();
+ dbgs() << "}\n";
+ }
+};
+
+// Represents "dependency" or "use" graph of global objects (functions and
+// global variables) in a module. It is used during device code split to
+// understand which global variables and functions (other than entry points)
+// should be included into a split module.
+//
+// Nodes of the graph represent LLVM's GlobalObjects, edges "A" -> "B" represent
+// the fact that if "A" is included into a module, then "B" should be included
+// as well.
+//
+// Examples of dependencies which are represented in this graph:
+// - Function FA calls function FB
+// - Function FA uses global variable GA
+// - Global variable GA references (initialized with) function FB
+// - Function FA stores address of a function FB somewhere
+//
+// The following cases are treated as dependencies between global objects:
+// 1. Global object A is used within by a global object B in any way (store,
+// bitcast, phi node, call, etc.): "A" -> "B" edge will be added to the
+// graph;
+// 2. function A performs an indirect call of a function with signature S and
+// there is a function B with signature S. "A" -> "B" edge will be added to
+// the graph;
+class DependencyGraph {
+public:
+ using GlobalSet = SmallPtrSet<const GlobalValue *, 16>;
+
+ DependencyGraph(const Module &M) {
+ // Group functions by their signature to handle case (2) described above
+ DenseMap<const FunctionType *, DependencyGraph::GlobalSet>
+ FuncTypeToFuncsMap;
+ for (const auto &F : M.functions()) {
+ // Kernels can't be called (either directly or indirectly) in SYCL
+ if (isKernel(F))
+ continue;
+
+ FuncTypeToFuncsMap[F.getFunctionType()].insert(&F);
+ }
+
+ for (const auto &F : M.functions()) {
+ // case (1), see comment above the class definition
+ for (const Value *U : F.users())
+ addUserToGraphRecursively(cast<const User>(U), &F);
+
+ // case (2), see comment above the class definition
+ for (const auto &I : instructions(F)) {
+ const auto *CI = dyn_cast<CallInst>(&I);
+ if (!CI || !CI->isIndirectCall()) // Direct calls were handled above
+ continue;
+
+ const FunctionType *Signature = CI->getFunctionType();
+ const auto &PotentialCallees = FuncTypeToFuncsMap[Signature];
+ Graph[&F].insert(PotentialCallees.begin(), PotentialCallees.end());
+ }
+ }
+
+ // And every global variable (but their handling is a bit simpler)
+ for (const auto &GV : M.globals())
+ for (const Value *U : GV.users())
+ addUserToGraphRecursively(cast<const User>(U), &GV);
+ }
+
+ iterator_range<GlobalSet::const_iterator>
+ dependencies(const GlobalValue *Val) const {
+ auto It = Graph.find(Val);
+ return (It == Graph.end())
+ ? make_range(EmptySet.begin(), EmptySet.end())
+ : make_range(It->second.begin(), It->second.end());
+ }
+
+private:
+ void addUserToGraphRecursively(const User *Root, const GlobalValue *V) {
+ SmallVector<const User *, 8> WorkList;
+ WorkList.push_back(Root);
+
+ while (!WorkList.empty()) {
+ const User *U = WorkList.pop_back_val();
+ if (const auto *I = dyn_cast<const Instruction>(U)) {
+ const auto *UFunc = I->getFunction();
+ Graph[UFunc].insert(V);
+ } else if (isa<const Constant>(U)) {
+ if (const auto *GV = dyn_cast<const GlobalVariable>(U))
+ Graph[GV].insert(V);
+ // This could be a global variable or some constant expression (like
+ // bitcast or gep). We trace users of this constant further to reach
+ // global objects they are used by and add them to the graph.
+ for (const auto *UU : U->users())
+ WorkList.push_back(UU);
+ } else
+ llvm_unreachable("Unhandled type of function user");
+ }
+ }
+
+ DenseMap<const GlobalValue *, GlobalSet> Graph;
+ SmallPtrSet<const GlobalValue *, 1> EmptySet;
+};
+
+void collectFunctionsAndGlobalVariablesToExtract(
+ SetVector<const GlobalValue *> &GVs, const Module &M,
+ const EntryPointGroup &ModuleEntryPoints, const DependencyGraph &DG) {
+ // We start with module entry points
+ for (const auto *F : ModuleEntryPoints.Functions)
+ GVs.insert(F);
+
+ // Non-discardable global variables are also include into the initial set
+ for (const auto &GV : M.globals())
+ if (!GV.isDiscardableIfUnused())
+ GVs.insert(&GV);
+
+ // GVs has SetVector type. This type inserts a value only if it is not yet
+ // present there. So, recursion is not expected here.
+ size_t Idx = 0;
+ while (Idx < GVs.size()) {
+ const GlobalValue *Obj = GVs[Idx++];
+
+ for (const GlobalValue *Dep : DG.dependencies(Obj)) {
+ if (const auto *Func = dyn_cast<const Function>(Dep)) {
+ if (!Func->isDeclaration())
+ GVs.insert(Func);
+ } else
+ GVs.insert(Dep); // Global variables are added unconditionally
+ }
+ }
+}
+
+ModuleDesc extractSubModule(const ModuleDesc &MD,
+ const SetVector<const GlobalValue *> &GVs,
+ EntryPointGroup ModuleEntryPoints) {
+ const Module &M = MD.getModule();
+ // For each group of entry points collect all dependencies.
+ ValueToValueMapTy VMap;
+ // Clone definitions only for needed globals. Others will be added as
+ // declarations and removed later.
+ std::unique_ptr<Module> SubM = CloneModule(
+ M, VMap, [&](const GlobalValue *GV) { return GVs.count(GV); });
+ // Replace entry points with cloned ones.
+ EntryPointSet NewEPs;
+ const EntryPointSet &EPs = ModuleEntryPoints.Functions;
+ std::for_each(EPs.begin(), EPs.end(), [&](const Function *F) {
+ NewEPs.insert(cast<Function>(VMap[F]));
+ });
+ ModuleEntryPoints.Functions = std::move(NewEPs);
+ return ModuleDesc{std::move(SubM), std::move(ModuleEntryPoints)};
+}
+
+// The function produces a copy of input LLVM IR module M with only those
+// functions and globals that can be called from entry points that are specified
+// in ModuleEntryPoints vector, in addition to the entry point functions.
+ModuleDesc extractCallGraph(const ModuleDesc &MD,
+ EntryPointGroup ModuleEntryPoints,
+ const DependencyGraph &DG) {
+ SetVector<const GlobalValue *> GVs;
+ collectFunctionsAndGlobalVariablesToExtract(GVs, MD.getModule(),
+ ModuleEntryPoints, DG);
+
+ ModuleDesc SplitM = extractSubModule(MD, GVs, std::move(ModuleEntryPoints));
+ LLVM_DEBUG(SplitM.dump());
+ SplitM.cleanup();
+ return SplitM;
+}
+
+using EntryPointGroupVec = SmallVector<EntryPointGroup, 0>;
+
+/// Module Splitter.
+/// It gets a module (in a form of module descriptor, to get additional info)
+/// and a collection of entry points groups. Each group specifies subset entry
+/// points from input module that should be included in a split module.
+class ModuleSplitter {
+private:
+ ModuleDesc Input;
+ EntryPointGroupVec Groups;
+ DependencyGraph DG;
+
+private:
+ EntryPointGroup drawEntryPointGroup() {
+ assert(Groups.size() > 0 && "Reached end of entry point groups list.");
+ EntryPointGroup Group = std::move(Groups.back());
+ Groups.pop_back();
+ return Group;
+ }
+
+public:
+ ModuleSplitter(ModuleDesc MD, EntryPointGroupVec GroupVec)
+ : Input(std::move(MD)), Groups(std::move(GroupVec)),
+ DG(Input.getModule()) {
+ assert(!Groups.empty() && "Entry points groups collection is empty!");
+ }
+
+ /// Gets next subsequence of entry points in an input module and provides
+ /// split submodule containing these entry points and their dependencies.
+ ModuleDesc getNextSplit() {
+ return extractCallGraph(Input, drawEntryPointGroup(), DG);
+ }
+
+ /// Check that there are still submodules to split.
+ bool hasMoreSplits() const { return Groups.size() > 0; }
+};
+
+} // namespace
+
+/// Gets attached attribute value if it is present. Otherwise returns empty
+/// stirng.
+static StringRef computeFunctionCategoryFromStringMetadata(const Function &F,
+ StringRef AttrName) {
+ return F.getFnAttribute(AttrName).getValueAsString();
+}
+
+static EntryPointGroupVec selectEntryPointGroups(const Module &M,
+ IRSplitMode Mode) {
+ // std::map is used here to ensure stable ordering of entry point groups,
+ // which is based on their contents, this greatly helps LIT tests
+ std::map<std::string, EntryPointSet> EntryPointsMap;
+
+ static constexpr char ATTR_SYCL_MODULE_ID[] = "sycl-module-id";
+ for (const auto &F : M.functions()) {
+ if (!isEntryPoint(F))
+ continue;
+
+ std::string Key;
+ switch (Mode) {
+ case IRSplitMode::IRSM_PER_KERNEL:
+ Key = F.getName();
+ break;
+ case IRSplitMode::IRSM_PER_TU:
+ Key = computeFunctionCategoryFromStringMetadata(F, ATTR_SYCL_MODULE_ID);
+ break;
+ case IRSplitMode::IRSM_NONE:
+ llvm_unreachable("");
+ }
+
+ EntryPointsMap[Key].insert(&F);
+ }
+
+ EntryPointGroupVec Groups;
+ if (EntryPointsMap.empty()) {
+ // No entry points met, record this.
+ Groups.emplace_back("-", EntryPointSet());
+ } else {
+ Groups.reserve(EntryPointsMap.size());
+ // Start with properties of a source module
+ for (auto &[Key, EntryPoints] : EntryPointsMap)
+ Groups.emplace_back(Key, std::move(EntryPoints));
+ }
+
+ return Groups;
+}
+
+static Error saveModuleIRInFile(Module &M, StringRef FilePath,
+ bool OutputAssembly) {
+ int FD = -1;
+ if (std::error_code EC = sys::fs::openFileForWrite(FilePath, FD))
+ return errorCodeToError(EC);
+
+ raw_fd_ostream OS(FD, true);
+ ModulePassManager MPM;
+ ModuleAnalysisManager MAM;
+ MAM.registerPass([&] { return PassInstrumentationAnalysis(); });
+ if (OutputAssembly)
+ MPM.addPass(PrintModulePass(OS));
+ else
+ MPM.addPass(BitcodeWriterPass(OS));
+
+ MPM.run(M, MAM);
+ return Error::success();
+}
+
+static Expected<SYCLSplitModule>
+saveModuleDesc(ModuleDesc &MD, std::string Prefix, bool OutputAssembly) {
+ Prefix += OutputAssembly ? ".ll" : ".bc";
+ if (Error E = saveModuleIRInFile(MD.getModule(), Prefix, OutputAssembly))
+ return E;
+
+ SYCLSplitModule SM;
+ SM.ModuleFilePath = Prefix;
+ SM.Symbols = MD.makeSymbolTable();
+ return SM;
+}
+
+namespace llvm {
+
+Expected<SmallVector<SYCLSplitModule, 0>>
+parseSYCLSplitModulesFromFile(StringRef File) {
+ auto EntriesMBOrErr = llvm::MemoryBuffer::getFile(File);
+ if (!EntriesMBOrErr)
+ return createFileError(File, EntriesMBOrErr.getError());
+
+ line_iterator LI(**EntriesMBOrErr);
+ if (LI.is_at_eof() || *LI != "[Code|Symbols]")
+ return createStringError(inconvertibleErrorCode(),
+ "invalid SYCL Table file.");
+
+ // "Code" and "Symbols" at the moment.
+ static constexpr int NUMBER_COLUMNS = 2;
+ ++LI;
+ SmallVector<SYCLSplitModule,...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Hi @jhuber6 @jdoerfert! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About the file naming - should it be SplitSYCLModule.cpp
? It implements the splitSYCLModule
API, and I think it fits better with the similarly named SplitModule.cpp
in the same directory. Regardless of where the SYCL
goes, I think having Split
before Module
keeps things somewhat consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly just nits from me
llvm/test/tools/llvm-split/SYCL/device-code-split/auto-module-split-1.ll
Outdated
Show resolved
Hide resolved
llvm/test/tools/llvm-split/SYCL/device-code-split/auto-module-split-1.ll
Outdated
Show resolved
Hide resolved
llvm/test/tools/llvm-split/SYCL/device-code-split/auto-module-split-1.ll
Outdated
Show resolved
Hide resolved
Note: more test's refactoring is coming. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New changes look good. Thanks!
One minor suggestion to make assert checks more readable.
…d from the first PR
Add source split mode checks. Remove unnecessary metadata.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job on making tests more maintainable! 👍
Overall looks good to me with a few nits in addition to the previous comment.
llvm/test/tools/llvm-split/SYCL/device-code-split/complex-indirect-call-chain.ll
Outdated
Show resolved
Hide resolved
; CHECK-SYM0: kernel2 | ||
; CHECK-SYM1: kernel1 | ||
; | ||
; CHECK-IR0: define dso_local spir_kernel void @kernel2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense to rename kernel2 to kernel0 to align with the check prefixes (or use 2 in the prefix name instead of 0).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I use letter 'A', 'B' for kernels.
; CHECK-MODULE1-NOT: define dso_local spir_kernel void @TU0_kernel0 | ||
; CHECK-MODULE1-TXT-NOT: TU0_kernel0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we add CHECK-MODULE0-NOT
and CHECK-MODULE0-TXT-NOT
for @TU0_kernel0?
BTW, I would sort the checks in ascending order (as the run lines).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added missing checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine, one nit.
} | ||
|
||
std::string makeSymbolTable() const { | ||
SmallString<128> ST; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest
SmallString<0> Data;
raw_svector_ostream OS(Data);
OS << F->getName() << "\n";
And just return the SmallString<0>
unless you really need the std::string
.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/1065 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/203/builds/4254 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/204/builds/3066 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/205/builds/3045 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/138/builds/11476 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/14413 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/20001 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/18363 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/16128 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/13365 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/181/builds/15241 Here is the relevant piece of the build log for the reference
|
Landed and reverted, please fix whatever the bots are complaining about and make a new PR. |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/14410 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/22132 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/13135 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/53/builds/13638 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/133/builds/12625 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/12933 Here is the relevant piece of the build log for the reference
|
This patch adds SYCL Module splitting - the necessary step in the SYCL compilation pipeline. Only 2 splitting modes are being added in this patch: by kernel and by source. The previous attempt was at llvm#119713. In this patch there is no dependency in `TransformUtils` on "IPO" and on "Printing Passes". In this patch a module splitting is self-contained and it doesn't introduce linking issues.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/2/builds/18973 Here is the relevant piece of the build log for the reference
|
This patch adds SYCL Module splitting - the necessary step in the SYCL compilation pipeline. Only 2 splitting modes are being added in this patch: by kernel and by source.