Skip to content
Draft
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 18 additions & 24 deletions clang/test/Driver/clang-sycl-linker-test.cpp
Original file line number Diff line number Diff line change
@@ -1,41 +1,35 @@
// Tests the clang-sycl-linker tool.
//
// Test a simple case without arguments.
// RUN: %clangxx -emit-llvm -c %s -o %t_1.bc
// RUN: %clangxx -emit-llvm -c %s -o %t_2.bc
// RUN: clang-sycl-linker --dry-run -triple spirv64 %t_1.bc %t_2.bc -o a.spv 2>&1 \
// RUN: | FileCheck %s --check-prefix=SIMPLE
// SIMPLE: "{{.*}}llvm-link{{.*}}" {{.*}}.bc {{.*}}.bc -o [[FIRSTLLVMLINKOUT:.*]].bc --suppress-warnings
// SIMPLE-NEXT: "{{.*}}llvm-spirv{{.*}}" {{.*}}-o a.spv [[FIRSTLLVMLINKOUT]].bc
//
// Test that llvm-link is not called when only one input is present.
// RUN: clang-sycl-linker --dry-run -triple spirv64 %t_1.bc -o a.spv 2>&1 \
// RUN: | FileCheck %s --check-prefix=SIMPLE-NO-LINK
// SIMPLE-NO-LINK: "{{.*}}llvm-spirv{{.*}}" {{.*}}-o a.spv {{.*}}.bc
// Test a simple case to link two input files.
// RUN: touch %t_1.o
// RUN: touch %t_2.o
// RUN: clang-sycl-linker --dry-run -triple spirv64 %t_1.o %t_2.o -o a.spv 2>&1 \
// RUN: | FileCheck %s --check-prefix=SIMPLE-FO
// SIMPLE-FO: sycl-device-link: inputs: {{.*}}.o, {{.*}}.o libfiles: output: [[LLVMLINKOUT:.*]].bc
// SIMPLE-FO-NEXT: "{{.*}}llvm-spirv{{.*}}" {{.*}}-o a.spv [[LLVMLINKOUT]].bc
//
// Test a simple case with device library files specified.
// RUN: touch %T/lib1.bc
// RUN: touch %T/lib2.bc
// RUN: clang-sycl-linker --dry-run -triple spirv64 %t_1.bc %t_2.bc --library-path=%T --device-libs=lib1.bc,lib2.bc -o a.spv 2>&1 \
// RUN: touch %T/lib1.o
// RUN: touch %T/lib2.o
// RUN: clang-sycl-linker --dry-run -triple spirv64 %t_1.o %t_2.o --library-path=%T --device-libs=lib1.o,lib2.o -o a.spv 2>&1 \
// RUN: | FileCheck %s --check-prefix=DEVLIBS
// DEVLIBS: "{{.*}}llvm-link{{.*}}" {{.*}}.bc {{.*}}.bc -o [[FIRSTLLVMLINKOUT:.*]].bc --suppress-warnings
// DEVLIBS-NEXT: "{{.*}}llvm-link{{.*}}" -only-needed [[FIRSTLLVMLINKOUT]].bc {{.*}}lib1.bc {{.*}}lib2.bc -o [[SECONDLLVMLINKOUT:.*]].bc --suppress-warnings
// DEVLIBS-NEXT: "{{.*}}llvm-spirv{{.*}}" {{.*}}-o a.spv [[SECONDLLVMLINKOUT]].bc
// DEVLIBS: sycl-device-link: inputs: {{.*}}.o libfiles: {{.*}}lib1.o, {{.*}}lib2.o output: [[LLVMLINKOUT:.*]].bc
// DEVLIBS-NEXT: "{{.*}}llvm-spirv{{.*}}" {{.*}}-o a.spv [[LLVMLINKOUT]].bc
//
// Test a simple case with .o (fat object) as input.
// TODO: Remove this test once fat object support is added.
// RUN: %clangxx -c %s -o %t.o
// RUN: not clang-sycl-linker --dry-run -triple spirv64 %t.o -o a.spv 2>&1 \
// Test a simple case with a random file (not bitcode) as input.
// RUN: touch %t.x.o
// RUN: clang-offload-packager -o %t.o --image=file=%t.x.o,kind=openmp,triple=spirv64,arch=
// RUN: not clang-sycl-linker -triple spirv64 %t.o -o a.spv 2>&1 \
// RUN: | FileCheck %s --check-prefix=FILETYPEERROR
// FILETYPEERROR: Unsupported file type
//
// Test to see if device library related errors are emitted.
// RUN: not clang-sycl-linker --dry-run -triple spirv64 %t_1.bc %t_2.bc --library-path=%T --device-libs= -o a.spv 2>&1 \
// RUN: | FileCheck %s --check-prefix=DEVLIBSERR1
// DEVLIBSERR1: Number of device library files cannot be zero
// RUN: not clang-sycl-linker --dry-run -triple spirv64 %t_1.bc %t_2.bc --library-path=%T --device-libs=lib1.bc,lib2.bc,lib3.bc -o a.spv 2>&1 \
// RUN: not clang-sycl-linker --dry-run -triple spirv64 %t_1.bc %t_2.bc --library-path=%T --device-libs=lib1.o,lib2.o,lib3.o -o a.spv 2>&1 \
// RUN: | FileCheck %s --check-prefix=DEVLIBSERR2
// DEVLIBSERR2: '{{.*}}lib3.bc' SYCL device library file is not found
// DEVLIBSERR2: '{{.*}}lib3.o' SYCL device library file is not found
//
// Test if correct set of llvm-spirv options are emitted for windows environment.
// RUN: clang-sycl-linker --dry-run -triple spirv64 --is-windows-msvc-env %t_1.bc %t_2.bc -o a.spv 2>&1 \
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Driver/sycl-link-spirv-target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
// Test that -Xlinker options are being passed to clang-sycl-linker.
// RUN: touch %t.bc
// RUN: %clangxx -### --target=spirv64 --sycl-link -Xlinker --llvm-spirv-path=/tmp \
// RUN: -Xlinker --library-path=/tmp -Xlinker --device-libs=lib1.bc,lib2.bc %t.bc 2>&1 \
// RUN: -Xlinker --library-path=/tmp -Xlinker --device-libs=lib1.o,lib2.o %t.bc 2>&1 \
// RUN: | FileCheck %s -check-prefix=XLINKEROPTS
// XLINKEROPTS: "{{.*}}clang-sycl-linker{{.*}}" "--llvm-spirv-path=/tmp" "--library-path=/tmp" "--device-libs=lib1.bc,lib2.bc" "{{.*}}.bc" "-o" "a.out"
// XLINKEROPTS: "{{.*}}clang-sycl-linker{{.*}}" "--llvm-spirv-path=/tmp" "--library-path=/tmp" "--device-libs=lib1.o,lib2.o" "{{.*}}.bc" "-o" "a.out"
4 changes: 4 additions & 0 deletions clang/tools/clang-sycl-linker/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
set(LLVM_LINK_COMPONENTS
${LLVM_TARGETS_TO_BUILD}
BinaryFormat
BitWriter
Core
IRReader
Linker
Option
Object
TargetParser
Expand Down
190 changes: 112 additions & 78 deletions clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
#include "llvm/Bitcode/BitcodeWriter.h"
#include "llvm/CodeGen/CommandFlags.h"
#include "llvm/IR/DiagnosticPrinter.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IRReader/IRReader.h"
#include "llvm/LTO/LTO.h"
#include "llvm/Linker/Linker.h"
#include "llvm/Object/Archive.h"
#include "llvm/Object/ArchiveWriter.h"
#include "llvm/Object/Binary.h"
Expand Down Expand Up @@ -180,7 +182,7 @@ Error executeCommands(StringRef ExecutablePath, ArrayRef<StringRef> Args) {
}

Expected<SmallVector<std::string>> getInput(const ArgList &Args) {
// Collect all input bitcode files to be passed to llvm-link.
// Collect all input bitcode files to be passed to the device linking stage.
SmallVector<std::string> BitcodeFiles;
for (const opt::Arg *Arg : Args.filtered(OPT_INPUT)) {
std::optional<std::string> Filename = std::string(Arg->getValue());
Expand All @@ -190,49 +192,48 @@ Expected<SmallVector<std::string>> getInput(const ArgList &Args) {
file_magic Magic;
if (auto EC = identify_magic(*Filename, Magic))
return createStringError("Failed to open file " + *Filename);
// TODO: Current use case involves LLVM IR bitcode files as input.
// This will be extended to support objects and SPIR-V IR files.
if (Magic != file_magic::bitcode)
return createStringError("Unsupported file type");
BitcodeFiles.push_back(*Filename);
}
return BitcodeFiles;
}

/// Link all SYCL device input files into one before adding device library
/// files. Device linking is performed using llvm-link tool.
/// 'InputFiles' is the list of all LLVM IR device input files.
/// 'Args' encompasses all arguments required for linking device code and will
/// be parsed to generate options required to be passed into llvm-link.
Expected<StringRef> linkDeviceInputFiles(ArrayRef<std::string> InputFiles,
const ArgList &Args) {
llvm::TimeTraceScope TimeScope("SYCL LinkDeviceInputFiles");

assert(InputFiles.size() && "No inputs to llvm-link");
// Early check to see if there is only one input.
if (InputFiles.size() < 2)
return InputFiles[0];

Expected<std::string> LLVMLinkPath =
findProgram(Args, "llvm-link", {getMainExecutable("llvm-link")});
if (!LLVMLinkPath)
return LLVMLinkPath.takeError();

SmallVector<StringRef> CmdArgs;
CmdArgs.push_back(*LLVMLinkPath);
for (auto &File : InputFiles)
CmdArgs.push_back(File);
// Create a new file to write the linked device file to.
auto OutFileOrErr =
createTempFile(Args, sys::path::filename(OutputFile), "bc");
if (!OutFileOrErr)
return OutFileOrErr.takeError();
CmdArgs.push_back("-o");
CmdArgs.push_back(*OutFileOrErr);
CmdArgs.push_back("--suppress-warnings");
if (Error Err = executeCommands(*LLVMLinkPath, CmdArgs))
Expected<std::unique_ptr<Module>> getBitcodeModule(StringRef File,
LLVMContext &C) {
SMDiagnostic Err;

// Handle cases where input file is a LLVM IR bitcode file.
// When clang-sycl-linker is called via clang-linker-wrapper tool, input files
// are LLVM IR bitcode files
// TODO: Support SPIR-V IR files
auto M = getLazyIRFileModule(File, Err, C);
if (M)
return std::move(M);

// Handle cases where input file is a fat object containing LLVM IR bitcode
// SYCL device libraries are provided as fat objects containing LLVM IR
// bitcode
Copy link

Choose a reason for hiding this comment

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

Why SYCL device libraries are passed to the clang-sycl-linker via command line arguments? Doesn't the tool itself know the default location and default library names to link? I'm okay to let the user to customize these parameters via options, but I see that clang-sycl-linker can't link device libraries unless they are configured from outside.

If they are configured from outside, I would argue that device libraries must be LLVM bitcode format, not fat objects. AFAIK, they are distributed in the LLVM bitcode format. @mdtoguchi, am I right?

Anyway, processing two different sets of files in one function doesn't look like a good approach in this case. User inputs are processed by the code at the beginning of the function. Basically, we have two separate functions glued together into one. Instead of separating these two functions, I suggest using LLVM bitcode format for the device libraries.

Copy link
Owner Author

Choose a reason for hiding this comment

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

This function gives us the ability to handle both bitcode files and bitcode files embedded in fat objects. I would like to keep it like this to give some flexibility on how inputs are sent into clang-sycl-linker. We can do a cleanup once all pieces are put together.

Hope that's alright. I looked into the /install directory and I see .o .spv and .bc files. With wait for @mdtoguchi to chime in here about this.

Thanks

Copy link

Choose a reason for hiding this comment

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

This function gives us the ability to handle both bitcode files and bitcode files embedded in fat objects. I would like to keep it like this to give some flexibility on how inputs are sent into clang-sycl-linker. We can do a cleanup once all pieces are put together.

This doesn't make sense to me. User inputs should never be fat objects.

Copy link
Collaborator

Choose a reason for hiding this comment

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

AFAIK, there was agreement that the device libraries should be known by the tools and not required to be passed in from the driver or from the user calling clang-linker-wrapper directly. I would also share Alexey's view that the device libraries should be .bc files. We have full control over the device libraries so any potential flexibility given shouldn't be needed.

One interesting note though, current usage of --offload-new-driver will pass in fat objects for the device libraries instead of .bc files like we do for the old model. I recall that @asudarsa made this distinction for some ease of use in the tool.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ok. I think we can use .bc files in new offload model as well. For now, I will continue to rely on the options for the device libraries to be passed in via options. Further discussions may be warranted on how we want to upstream the device library sources and build them in the upstream compiler.

Thanks

ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
MemoryBuffer::getFile(File);
if (std::error_code EC = BufferOrErr.getError())
return createFileError(File, EC);
MemoryBufferRef Buffer = **BufferOrErr;
SmallVector<OffloadFile> Binaries;
if (Error Err = extractOffloadBinaries(Buffer, Binaries))
return std::move(Err);
return Args.MakeArgString(*OutFileOrErr);
// Only one binary of bitcode image type is expected
assert((Binaries.size() == 1) && "Only one binary per file is expected");
auto Bin = std::move(Binaries.front());
// TODO: Current use case involves LLVM IR bitcode files as input.
// This will be extended to support objects and SPIR-V IR files.
auto TheImage = Bin.getBinary()->getImage();
if (identify_magic(TheImage) != file_magic::bitcode)
return createStringError("Unsupported file type");
// TODO: Try to replace this call with getLazyIRModule
M = parseIR(MemoryBufferRef(TheImage, ""), Err, C);
if (M)
return std::move(M);

return createStringError("Unsupported file type");
}

// This utility function is used to gather all SYCL device library files that
Expand Down Expand Up @@ -264,44 +265,81 @@ Expected<SmallVector<std::string>> getSYCLDeviceLibs(const ArgList &Args) {
return DeviceLibFiles;
}

/// Link all device library files and input file into one LLVM IR file. This
/// linking is performed using llvm-link tool.
/// 'InputFiles' is the list of all LLVM IR device input files.
/// 'Args' encompasses all arguments required for linking device code and will
/// be parsed to generate options required to be passed into llvm-link tool.
static Expected<StringRef> linkDeviceLibFiles(StringRef InputFile,
const ArgList &Args) {
llvm::TimeTraceScope TimeScope("LinkDeviceLibraryFiles");
/// Following tasks are performed:
/// 1. Link all SYCL device bitcode images into one image. Device linking is
/// performed using the linkInModule API.
/// 2. Gather all SYCL device library bitcode images.
/// 3. Link all the images gathered in Step 2 with the output of Step 1 using
/// linkInModule API. LinkOnlyNeeded flag is used.
Expected<StringRef> linkDeviceCode(ArrayRef<std::string> InputFiles,
const ArgList &Args) {
llvm::TimeTraceScope TimeScope("SYCL link device code");
LLVMContext C;

assert(InputFiles.size() && "No inputs to link");

// Create a new file to write the linked device file to
auto BitcodeOutput =
createTempFile(Args, sys::path::filename(OutputFile), "bc");
if (!BitcodeOutput)
return BitcodeOutput.takeError();

// Get all SYCL device library files, if any
auto SYCLDeviceLibFiles = getSYCLDeviceLibs(Args);
if (!SYCLDeviceLibFiles)
return SYCLDeviceLibFiles.takeError();
if ((*SYCLDeviceLibFiles).empty())
return InputFile;

Expected<std::string> LLVMLinkPath =
findProgram(Args, "llvm-link", {getMainExecutable("llvm-link")});
if (!LLVMLinkPath)
return LLVMLinkPath.takeError();
if (DryRun || Verbose) {
std::string Inputs =
std::accumulate(std::next(InputFiles.begin()), InputFiles.end(),
InputFiles.front(), [](std::string a, std::string b) {
return std::move(a) + ", " + std::move(b);
});
std::string LibInputs = "";
if (!(*SYCLDeviceLibFiles).empty())
LibInputs = std::accumulate(
std::next((*SYCLDeviceLibFiles).begin()), (*SYCLDeviceLibFiles).end(),
(*SYCLDeviceLibFiles).front(), [](std::string a, std::string b) {
return std::move(a) + ", " + std::move(b);
});
errs() << formatv(
"sycl-device-link: inputs: {0} libfiles: {1} output: {2}\n", Inputs,
LibInputs, *BitcodeOutput);
if (DryRun)
return *BitcodeOutput;
}

// Create a new file to write the linked device file to.
auto OutFileOrErr =
createTempFile(Args, sys::path::filename(OutputFile), "bc");
if (!OutFileOrErr)
return OutFileOrErr.takeError();
auto LinkerOutput = std::make_unique<Module>("sycl-device-link", C);
Linker L(*LinkerOutput);
// Link SYCL device input files.
for (auto &File : InputFiles) {
auto ModOrErr = getBitcodeModule(File, C);
if (!ModOrErr)
return ModOrErr.takeError();
if (L.linkInModule(std::move(*ModOrErr)))
return createStringError("Could not link IR");
}

SmallVector<StringRef, 8> CmdArgs;
CmdArgs.push_back(*LLVMLinkPath);
CmdArgs.push_back("-only-needed");
CmdArgs.push_back(InputFile);
for (auto &File : *SYCLDeviceLibFiles)
CmdArgs.push_back(File);
CmdArgs.push_back("-o");
CmdArgs.push_back(*OutFileOrErr);
CmdArgs.push_back("--suppress-warnings");
if (Error Err = executeCommands(*LLVMLinkPath, CmdArgs))
return std::move(Err);
return *OutFileOrErr;
// Link in SYCL device library files.
const llvm::Triple Triple(Args.getLastArgValue(OPT_triple));
for (auto &File : *SYCLDeviceLibFiles) {
auto LibMod = getBitcodeModule(File, C);
if (!LibMod)
return LibMod.takeError();
if ((*LibMod)->getTargetTriple() == Triple) {
unsigned Flags = Linker::Flags::None;
if (L.linkInModule(std::move(*LibMod), Flags))
return createStringError("Could not link IR");
}
}

// Write the final output into 'BitcodeOutput' file
int FD = -1;
if (std::error_code EC = sys::fs::openFileForWrite(*BitcodeOutput, FD))
return errorCodeToError(EC);
llvm::raw_fd_ostream OS(FD, true);
WriteBitcodeToFile(*LinkerOutput, OS);
return *BitcodeOutput;
}

/// Add any llvm-spirv option that relies on a specific Triple in addition
Expand Down Expand Up @@ -345,7 +383,7 @@ static void getSPIRVTransOpts(const ArgList &Args,
",+SPV_INTEL_arbitrary_precision_fixed_point"
",+SPV_INTEL_arbitrary_precision_floating_point"
",+SPV_INTEL_variable_length_array,+SPV_INTEL_fp_fast_math_mode"
",+SPV_INTEL_long_constant_composite"
",+SPV_INTEL_long_composites"
",+SPV_INTEL_arithmetic_fence"
",+SPV_INTEL_global_variable_decorations"
",+SPV_INTEL_cache_controls"
Expand Down Expand Up @@ -424,18 +462,14 @@ static Expected<StringRef> runLLVMToSPIRVTranslation(StringRef File,

Error runSYCLLink(ArrayRef<std::string> Files, const ArgList &Args) {
llvm::TimeTraceScope TimeScope("SYCLDeviceLink");
// First llvm-link step
auto LinkedFile = linkDeviceInputFiles(Files, Args);

// Link all input bitcode files and SYCL device library files, if any
auto LinkedFile = linkDeviceCode(Files, Args);
if (!LinkedFile)
reportError(LinkedFile.takeError());

// second llvm-link step
auto DeviceLinkedFile = linkDeviceLibFiles(*LinkedFile, Args);
if (!DeviceLinkedFile)
reportError(DeviceLinkedFile.takeError());

// LLVM to SPIR-V translation step
auto SPVFile = runLLVMToSPIRVTranslation(*DeviceLinkedFile, Args);
auto SPVFile = runLLVMToSPIRVTranslation(*LinkedFile, Args);
if (!SPVFile)
return SPVFile.takeError();
return Error::success();
Expand Down