Skip to content
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

[SYCL] PoC implementation of kernel compiler extension with libtooling and sycl-jit #15701

Open
wants to merge 14 commits into
base: sycl
Choose a base branch
from
Open
5 changes: 5 additions & 0 deletions sycl-jit/common/include/Kernel.h
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,11 @@ struct SYCLKernelInfo {
: Name{KernelName}, Args{NumArgs}, Attributes{}, NDR{}, BinaryInfo{} {}
};

struct IncludePair {
const char *Path;
const char *Contents;
};

} // namespace jit_compiler

#endif // SYCL_FUSION_COMMON_KERNEL_H
11 changes: 11 additions & 0 deletions sycl-jit/jit-compiler/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ add_llvm_library(sycl-jit
lib/fusion/FusionHelper.cpp
lib/fusion/JITContext.cpp
lib/fusion/ModuleHelper.cpp
lib/rtc/DeviceCompilation.cpp
lib/helper/ConfigHelper.cpp

SHARED
Expand All @@ -29,6 +30,14 @@ add_llvm_library(sycl-jit
TargetParser
MC
${LLVM_TARGETS_TO_BUILD}

LINK_LIBS
clangBasic
clangDriver
clangFrontend
clangCodeGen
clangTooling
clangSerialization
)

target_compile_options(sycl-jit PRIVATE ${SYCL_JIT_WARNING_FLAGS})
Expand All @@ -40,6 +49,8 @@ target_include_directories(sycl-jit
SYSTEM PRIVATE
${LLVM_MAIN_INCLUDE_DIR}
${LLVM_SPIRV_INCLUDE_DIRS}
${CMAKE_SOURCE_DIR}/../clang/include
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does LLVM CMake not define a variable for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this to use ${LLVM_EXTERNAL_CLANG_SOURCE_DIR}.

Unfortunately there doesn't seem to be an equivalent for the build directory, from which an .inc file that defines CLANG_VERSION_MAJOR is included.

${CMAKE_BINARY_DIR}/tools/clang/include
)
target_include_directories(sycl-jit
PUBLIC
Expand Down
3 changes: 3 additions & 0 deletions sycl-jit/jit-compiler/include/KernelFusion.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ JITResult materializeSpecConstants(const char *KernelName,
jit_compiler::SYCLKernelBinaryInfo &BinInfo,
View<unsigned char> SpecConstBlob);

JITResult compileSYCL(const char *SYCLSource, View<IncludePair> IncludePairs,
View<const char *> UserArgs, const char *DPCPPRoot);

/// Clear all previously set options.
void resetJITConfiguration();

Expand Down
1 change: 1 addition & 0 deletions sycl-jit/jit-compiler/ld-version-script.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/* Export the library entry points */
fuseKernels;
materializeSpecConstants;
compileSYCL;
resetJITConfiguration;
addToJITConfiguration;

Expand Down
26 changes: 26 additions & 0 deletions sycl-jit/jit-compiler/lib/KernelFusion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "fusion/FusionPipeline.h"
#include "helper/ConfigHelper.h"
#include "helper/ErrorHandling.h"
#include "rtc/DeviceCompilation.h"
#include "translation/KernelTranslation.h"
#include "translation/SPIRVLLVMTranslation.h"
#include <llvm/Support/Error.h>
Expand Down Expand Up @@ -235,6 +236,31 @@ extern "C" JITResult fuseKernels(View<SYCLKernelInfo> KernelInformation,
return JITResult{FusedKernelInfo};
}

extern "C" JITResult compileSYCL(const char *SYCLSource,
View<IncludePair> IncludePairs,
View<const char *> UserArgs,
const char *DPCPPRoot) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DPCPPRoot is not like the other arguments. Doesn't it seem like the routine should be able to figure that out itself, rather than being provided?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense; it's detected from inside the JIT library now.

std::unique_ptr<llvm::Module> Module =
compileDeviceCode(SYCLSource, IncludePairs, UserArgs, DPCPPRoot);
if (!Module) {
return JITResult{"Device code compilation failed"};
}

SYCLKernelInfo Kernel;
auto Error = translation::KernelTranslator::translateKernel(
Kernel, *Module, JITContext::getInstance(), BinaryFormat::SPIRV);

auto *LLVMCtx = &Module->getContext();
Module.reset();
delete LLVMCtx;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How expensive is it to set up and destroy the LLVMContext on every call to RTC? Would it be an alternative to use the context from JITContext?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not mistaken, certain things like constants and metadata are stored within LLVMContext and won't be deallocated even if a module which references them is deallocated. Therefore, keeping LLVMContext between RTC call invocations could result in some memory build-up.

At least that is the behavior we discovered a few years ago when we were debugging exceptionally huge memory footprint of sycl-post-link where we had a huge codebase compiled with per-kernel device code split. I don't know if anything has changed since that in the upstream LLVM, but we hadn't proposed any patches back then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's certainly possible to pass an existing context into the ToolAction, but that also raises questions of thread safety.

For the performance implications, a) yes, looks like setting up a context does involve a non-trivial amount of work, and b) still seems true that types, constants and metadata are allocated in the context and not freed when the module is destroyed. I'd propose to keep the simple implementation for now, and will look out for the context setup overhead once we start benchmarking the RTC.


if (Error) {
return errorToFusionResult(std::move(Error), "SPIR-V translation failed");
}

return JITResult{Kernel};
}

extern "C" void resetJITConfiguration() { ConfigHelper::reset(); }

extern "C" void addToJITConfiguration(OptionStorage &&Opt) {
Expand Down
111 changes: 111 additions & 0 deletions sycl-jit/jit-compiler/lib/rtc/DeviceCompilation.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
//==---------------------- DeviceCompilation.cpp ---------------------------==//
//
// 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
//
//===----------------------------------------------------------------------===//

#include "DeviceCompilation.h"

#include <clang/Basic/Version.h>
#include <clang/CodeGen/CodeGenAction.h>
#include <clang/Driver/Compilation.h>
#include <clang/Frontend/CompilerInstance.h>
#include <clang/Tooling/CompilationDatabase.h>
#include <clang/Tooling/Tooling.h>

#include <llvm/IR/Module.h>

namespace {
using namespace clang;
using namespace clang::tooling;
using namespace clang::driver;

struct GetLLVMModuleAction : public ToolAction {
// Code adapted from `FrontendActionFactory::runInvocation`.
bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
FileManager *Files,
std::shared_ptr<PCHContainerOperations> PCHContainerOps,
DiagnosticConsumer *DiagConsumer) override {
assert(!Module && "Action should only be invoked on a single file");

// Create a compiler instance to handle the actual work.
CompilerInstance Compiler(std::move(PCHContainerOps));
Compiler.setInvocation(std::move(Invocation));
Compiler.setFileManager(Files);

// Create the compiler's actual diagnostics engine.
Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
if (!Compiler.hasDiagnostics()) {
return false;
}

Compiler.createSourceManager(*Files);

// Ignore `Compiler.getFrontendOpts().ProgramAction` (would be `EmitBC`) and
// create/execute an `EmitLLVMOnlyAction` (= codegen to LLVM module without
// emitting anything) instead.
EmitLLVMOnlyAction ELOA;
const bool Success = Compiler.ExecuteAction(ELOA);
Files->clearStatCache();
if (!Success) {
return false;
}

// Take the module and its context to extend the objects' lifetime.
Module = ELOA.takeModule();
ELOA.takeLLVMContext();

return true;
}

std::unique_ptr<llvm::Module> Module;
};

} // anonymous namespace

std::unique_ptr<llvm::Module> jit_compiler::compileDeviceCode(
const char *SYCLSource, View<IncludePair> IncludePairs,
View<const char *> UserArgs, const char *DPCPPRoot) {

SmallVector<std::string> CommandLine = {"-fsycl-device-only"};
// TODO: Allow instrumentation again when device library linking is
// implemented.
CommandLine.push_back("-fno-sycl-instrument-device-code");
CommandLine.append(UserArgs.begin(), UserArgs.end());
clang::tooling::FixedCompilationDatabase DB{"./", CommandLine};

constexpr auto SourcePath = "rtc.cpp";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than hard-code "rtc.cpp" in, can this 'fantasy name' for the file be an argument to the API? It might show up in debug information, so it might be useful to users to be able to disambiguate, rather than having every dynamic device compiled kernel originate with the same fictional file name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, will do 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I'm passing in a semi-random ID, same as the file-based implementation. There's no property yet in the extension to specify a file name or prefix, correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the reason that a user might want to change this name? Why is it better to have a semi-random ID as a default rather than a fixed string?

I can imagine that the name might show up in error / log messages, for example, when there is a syntax error in the source string. If that's the only case the name is visible, it seems like having a fixed string like rtc.cpp would be fine, and probably preferable to a name with a random number.

I'm not opposed to adding a property which allows the user to set this name, but I think it should be an optional property because I think many people will not care what the name is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can imagine that the name might show up in error / log messages, for example, when there is a syntax error in the source string. If that's the only case the name is visible, [...]

Yes, that's the only case. I agree that the ID doesn't add much value here because we don't materialise anything on the actual filesystem. I'll keep the plumbing to pass the filename down to the JIT, but will set it rtc.cpp until there's a need and a means to modify it from the extension.

clang::tooling::ClangTool Tool{DB, {SourcePath}};

// Set up in-memory filesystem.
Tool.mapVirtualFile(SourcePath, SYCLSource);
for (const auto &IP : IncludePairs) {
Tool.mapVirtualFile(IP.Path, IP.Contents);
}

// Reset argument adjusters to drop the `-fsyntax-only` flag which is added by
// default by this API.
Tool.clearArgumentsAdjusters();
// Then, modify argv[0] and set the resource directory so that the driver
// picks up the correct SYCL environment.
Tool.appendArgumentsAdjuster(
[&DPCPPRoot](const CommandLineArguments &Args,
StringRef Filename) -> CommandLineArguments {
(void)Filename;
CommandLineArguments NewArgs = Args;
NewArgs[0] = (Twine(DPCPPRoot) + "/bin/clang++").str();
NewArgs.push_back((Twine("-resource-dir=") + DPCPPRoot + "/lib/clang/" +
Twine(CLANG_VERSION_MAJOR))
.str());
Comment on lines +136 to +138
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these paths also apply in a packaged release?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, a release icpx follows the same path structure (checked with -print-resource-dir).

return NewArgs;
});

GetLLVMModuleAction Action;
if (!Tool.run(&Action)) {
return std::move(Action.Module);
}

return {};
}
30 changes: 30 additions & 0 deletions sycl-jit/jit-compiler/lib/rtc/DeviceCompilation.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//==---- DeviceCompilation.h - Compile SYCL device code with libtooling ----==//
//
// 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
//
//===----------------------------------------------------------------------===//

#ifndef SYCL_JIT_COMPILER_RTC_DEVICE_COMPILATION_H
#define SYCL_JIT_COMPILER_RTC_DEVICE_COMPILATION_H

#include "Kernel.h"
#include "View.h"

#include <memory>

namespace llvm {
class Module;
} // namespace llvm

namespace jit_compiler {

std::unique_ptr<llvm::Module> compileDeviceCode(const char *SYCLSource,
View<IncludePair> IncludePairs,
View<const char *> UserArgs,
const char *DPCPPRoot);

} // namespace jit_compiler

#endif // SYCL_JIT_COMPILER_RTC_DEVICE_COMPILATION_H
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ SPIRV::TranslatorOpts &SPIRVLLVMTranslator::translatorOpts() {
// there's currently no obvious way to iterate the
// array of extensions in KernelInfo.
TransOpt.enableAllExtensions();
// TODO: Remove this workaround.
TransOpt.setAllowedToUseExtension(
SPIRV::ExtensionID::SPV_KHR_untyped_pointers, false);
TransOpt.setDesiredBIsRepresentation(
SPIRV::BIsRepresentation::SPIRVFriendlyIR);
// TODO: We need to take care of specialization constants, either by
Expand Down
67 changes: 67 additions & 0 deletions sycl/source/detail/jit_compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
#include <sycl/detail/ur.hpp>
#include <sycl/kernel_bundle.hpp>

#include <dlfcn.h>
#include <link.h>

namespace sycl {
inline namespace _V1 {
namespace detail {
Expand Down Expand Up @@ -74,6 +77,31 @@ jit_compiler::jit_compiler() {
return false;
}

this->CompileSYCLHandle = reinterpret_cast<CompileSYCLFuncT>(
sycl::detail::ur::getOsLibraryFuncAddress(LibraryPtr, "compileSYCL"));
if (!this->CompileSYCLHandle) {
printPerformanceWarning(
"Cannot resolve JIT library function entry point");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds more serious than a mere performance warning :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

printPerformanceWarning is the generic error message helper in sycl-jit, but yes, I agree it's a bit of a misnomer when used here (and while attempting to set-up the other entrypoints before).

return false;
}

// TODO: Move this query to a more appropriate location (e.g. add
// `sycl::detail::ur::getOsLibraryPath`), and handle non-POSIX OSs. For now,
// it should be fine here because the JIT is not built on Windows.
link_map *Map = nullptr;
if (dlinfo(LibraryPtr, RTLD_DI_LINKMAP, &Map) == 0) {
std::string LoadedLibraryPath = Map->l_name;
std::string JITLibraryPathSuffix = "/lib/" + JITLibraryName;
auto Pos = LoadedLibraryPath.rfind(JITLibraryPathSuffix);
if (Pos != std::string::npos) {
this->DPCPPRoot = LoadedLibraryPath.substr(0, Pos);
}
}
if (this->DPCPPRoot.empty()) {
printPerformanceWarning("Cannot determine JIT library location");
return false;
}

return true;
};
Available = checkJITLibrary();
Expand Down Expand Up @@ -1143,6 +1171,45 @@ std::vector<uint8_t> jit_compiler::encodeReqdWorkGroupSize(
return Encoded;
}

std::vector<uint8_t> jit_compiler::compileSYCL(
const std::string &SYCLSource,
const std::vector<std::pair<std::string, std::string>> &IncludePairs,
const std::vector<std::string> &UserArgs, std::string *LogPtr,
const std::vector<std::string> &RegisteredKernelNames) {

// TODO: Handle situation.
assert(RegisteredKernelNames.empty() &&
"Instantiation of kernel templates NYI");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we throw instead? In cases where asserts are disabled what would happen if execution continues from here?


std::vector<::jit_compiler::IncludePair> IncludePairsView;
IncludePairsView.reserve(IncludePairs.size());
std::transform(IncludePairs.begin(), IncludePairs.end(),
std::back_inserter(IncludePairsView), [](const auto &Pair) {
return ::jit_compiler::IncludePair{Pair.first.c_str(),
Pair.second.c_str()};
});
std::vector<const char *> UserArgsView;
UserArgsView.reserve(UserArgs.size());
std::transform(UserArgs.begin(), UserArgs.end(),
std::back_inserter(UserArgsView),
[](const auto &Arg) { return Arg.c_str(); });

auto Result = CompileSYCLHandle(SYCLSource.c_str(), IncludePairsView,
UserArgsView, DPCPPRoot.c_str());

if (Result.failed()) {
throw sycl::exception(sycl::errc::build, Result.getErrorMessage());
}

// TODO: We currently don't have a meaningful build log.
(void)LogPtr;

const auto &BI = Result.getKernelInfo().BinaryInfo;
assert(BI.Format == ::jit_compiler::BinaryFormat::SPIRV);
std::vector<uint8_t> SPV(BI.BinaryStart, BI.BinaryStart + BI.BinarySize);
return SPV;
}

} // namespace detail
} // namespace _V1
} // namespace sycl
Expand Down
9 changes: 9 additions & 0 deletions sycl/source/detail/jit_compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ class jit_compiler {
const std::string &KernelName,
const std::vector<unsigned char> &SpecConstBlob);

std::vector<uint8_t> compileSYCL(
const std::string &SYCLSource,
const std::vector<std::pair<std::string, std::string>> &IncludePairs,
const std::vector<std::string> &UserArgs, std::string *LogPtr,
const std::vector<std::string> &RegisteredKernelNames);

bool isAvailable() { return Available; }

static jit_compiler &get_instance() {
Expand Down Expand Up @@ -80,12 +86,15 @@ class jit_compiler {
using FuseKernelsFuncT = decltype(::jit_compiler::fuseKernels) *;
using MaterializeSpecConstFuncT =
decltype(::jit_compiler::materializeSpecConstants) *;
using CompileSYCLFuncT = decltype(::jit_compiler::compileSYCL) *;
using ResetConfigFuncT = decltype(::jit_compiler::resetJITConfiguration) *;
using AddToConfigFuncT = decltype(::jit_compiler::addToJITConfiguration) *;
FuseKernelsFuncT FuseKernelsHandle = nullptr;
MaterializeSpecConstFuncT MaterializeSpecConstHandle = nullptr;
CompileSYCLFuncT CompileSYCLHandle = nullptr;
ResetConfigFuncT ResetConfigHandle = nullptr;
AddToConfigFuncT AddToConfigHandle = nullptr;
std::string DPCPPRoot;
#endif // SYCL_EXT_JIT_ENABLE
};

Expand Down
6 changes: 6 additions & 0 deletions sycl/source/detail/kernel_bundle_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,12 @@ class kernel_bundle_impl {
BuildOptions, LogPtr,
RegisteredKernelNames);
}
if (Language == syclex::source_language::sycljit) {
const auto &SourceStr = std::get<std::string>(this->Source);
return syclex::detail::SYCLJIT_to_SPIRV(SourceStr, IncludePairs,
BuildOptions, LogPtr,
RegisteredKernelNames);
}
throw sycl::exception(
make_error_code(errc::invalid),
"OpenCL C and SPIR-V are the only supported languages at this time");
Expand Down
Loading
Loading