-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
[clang-sycl-linker] Replace llvm-link with API calls #133797
Conversation
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Arvind Sudarsanam (asudarsa) ChangesThis PR has the following changes: Replace llvm-link with calls to linkInModule to link device files Add -print-linked-module option to dump linked module for testing Added a test to verify that linking is working as expected. We will eventually move to using thin LTO for linking device inputs. Thanks Patch is 21.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133797.diff 10 Files Affected:
diff --git a/clang/test/Driver/Inputs/SYCL/bar.ll b/clang/test/Driver/Inputs/SYCL/bar.ll
new file mode 100644
index 0000000000000..d17221b8dca18
--- /dev/null
+++ b/clang/test/Driver/Inputs/SYCL/bar.ll
@@ -0,0 +1,7 @@
+target triple = "spirv64"
+
+define spir_func i32 @bar_func1(i32 %a, i32 %b) {
+entry:
+ %res = add nsw i32 %b, %a
+ ret i32 %res
+}
diff --git a/clang/test/Driver/Inputs/SYCL/baz.ll b/clang/test/Driver/Inputs/SYCL/baz.ll
new file mode 100644
index 0000000000000..6cdf3735ed77e
--- /dev/null
+++ b/clang/test/Driver/Inputs/SYCL/baz.ll
@@ -0,0 +1,15 @@
+target triple = "spirv64"
+
+define spir_func i32 @bar_func1(i32 %a, i32 %b) {
+entry:
+ %mul = shl nsw i32 %a, 1
+ %res = add nsw i32 %mul, %b
+ ret i32 %res
+}
+
+define spir_func i32 @baz_func1(i32 %a) {
+entry:
+ %add = add nsw i32 %a, 5
+ %res = tail call spir_func i32 @bar_func1(i32 %a, i32 %add)
+ ret i32 %res
+}
diff --git a/clang/test/Driver/Inputs/SYCL/foo.ll b/clang/test/Driver/Inputs/SYCL/foo.ll
new file mode 100644
index 0000000000000..43aaf1424ee2d
--- /dev/null
+++ b/clang/test/Driver/Inputs/SYCL/foo.ll
@@ -0,0 +1,19 @@
+target triple = "spirv64"
+
+define spir_func i32 @foo_func1(i32 %a, i32 %b) {
+entry:
+ %call = tail call spir_func i32 @addFive(i32 %b)
+ %res = tail call spir_func i32 @bar_func1(i32 %a, i32 %call)
+ ret i32 %res
+}
+
+declare spir_func i32 @bar_func1(i32, i32)
+
+declare spir_func i32 @addFive(i32)
+
+define spir_func i32 @foo_func2(i32 %c, i32 %d, i32 %e) {
+entry:
+ %call = tail call spir_func i32 @foo_func1(i32 %c, i32 %d)
+ %res = mul nsw i32 %call, %e
+ ret i32 %res
+}
diff --git a/clang/test/Driver/Inputs/SYCL/libsycl.ll b/clang/test/Driver/Inputs/SYCL/libsycl.ll
new file mode 100644
index 0000000000000..fdc4643e97b6a
--- /dev/null
+++ b/clang/test/Driver/Inputs/SYCL/libsycl.ll
@@ -0,0 +1,13 @@
+target triple = "spirv64"
+
+define spir_func i32 @addFive(i32 %a) {
+entry:
+ %res = add nsw i32 %a, 5
+ ret i32 %res
+}
+
+define spir_func i32 @unusedFunc(i32 %a) {
+entry:
+ %res = mul nsw i32 %a, 5
+ ret i32 %res
+}
diff --git a/clang/test/Driver/clang-sycl-linker-test.cpp b/clang/test/Driver/clang-sycl-linker-test.cpp
index f358900b4fbd8..729561bd09cd8 100644
--- a/clang/test/Driver/clang-sycl-linker-test.cpp
+++ b/clang/test/Driver/clang-sycl-linker-test.cpp
@@ -1,48 +1,41 @@
// 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 the dry run of a simple case to link two input files.
+// RUN: %clangxx -emit-llvm -c -target spirv64 %s -o %t_1.bc
+// RUN: %clangxx -emit-llvm -c -target spirv64 %s -o %t_2.bc
+// RUN: clang-sycl-linker --dry-run -v -triple=spirv64 %t_1.bc %t_2.bc -o a.spv 2>&1 \
+// RUN: | FileCheck %s --check-prefix=SIMPLE-FO
+// SIMPLE-FO: sycl-device-link: inputs: {{.*}}.bc, {{.*}}.bc libfiles: output: [[LLVMLINKOUT:.*]].bc
+// SIMPLE-FO-NEXT: "{{.*}}llvm-spirv{{.*}}" {{.*}}-o a.spv [[LLVMLINKOUT]].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 with device library files specified.
+// Test the dry run of 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: clang-sycl-linker --dry-run -v -triple=spirv64 %t_1.bc %t_2.bc --library-path=%T --device-libs=lib1.bc,lib2.bc -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: {{.*}}.bc libfiles: {{.*}}lib1.bc, {{.*}}lib2.bc 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.o
+// 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: 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.bc,lib2.bc,lib3.bc -o a.spv 2>&1 \
// RUN: | FileCheck %s --check-prefix=DEVLIBSERR2
// DEVLIBSERR2: '{{.*}}lib3.bc' 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 \
+// RUN: clang-sycl-linker --dry-run -v -triple=spirv64 --is-windows-msvc-env %t_1.bc %t_2.bc -o a.spv 2>&1 \
// RUN: | FileCheck %s --check-prefix=LLVMOPTSWIN
// LLVMOPTSWIN: -spirv-debug-info-version=ocl-100 -spirv-allow-extra-diexpressions -spirv-allow-unknown-intrinsics=llvm.genx. -spirv-ext=
//
// Test if correct set of llvm-spirv options are emitted for linux environment.
-// RUN: clang-sycl-linker --dry-run -triple spirv64 %t_1.bc %t_2.bc -o a.spv 2>&1 \
+// RUN: clang-sycl-linker --dry-run -v -triple=spirv64 %t_1.bc %t_2.bc -o a.spv 2>&1 \
// RUN: | FileCheck %s --check-prefix=LLVMOPTSLIN
// LLVMOPTSLIN: -spirv-debug-info-version=nonsemantic-shader-200 -spirv-allow-unknown-intrinsics=llvm.genx. -spirv-ext=
diff --git a/clang/test/Driver/link-device-code.test b/clang/test/Driver/link-device-code.test
new file mode 100644
index 0000000000000..fbbcc14ec7cdd
--- /dev/null
+++ b/clang/test/Driver/link-device-code.test
@@ -0,0 +1,23 @@
+# RUN: llvm-as %S/Inputs/SYCL/foo.ll -o %t.foo.bc
+# RUN: llvm-as %S/Inputs/SYCL/bar.ll -o %t.bar.bc
+# RUN: llvm-as %S/Inputs/SYCL/baz.ll -o %t.baz.bc
+# RUN: llvm-as %S/Inputs/SYCL/libsycl.ll -o %t.libsycl.bc
+# RUN: clang-sycl-linker %t.foo.bc %t.bar.bc -triple=spirv64 --dry-run -o a.spv --print-linked-module 2>&1 | FileCheck %s --check-prefix=CHECK-SIMPLE
+
+# RUN: not clang-sycl-linker %t.bar.bc %t.baz.bc -triple=spirv64 --dry-run -o a.spv --print-linked-module 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-DEFS
+
+# RUN: clang-sycl-linker %t.foo.bc %t.bar.bc -device-libs=%t.libsycl.bc -library-path=/ -triple=spirv64 --dry-run -o a.spv --print-linked-module 2>&1 | FileCheck %s --check-prefix=CHECK-DEVICE-LIB
+
+; CHECK-SIMPLE: define {{.*}}foo_func1{{.*}}
+; CHECK-SIMPLE: define {{.*}}foo_func2{{.*}}
+; CHECK-SIMPLE: define {{.*}}bar_func1{{.*}}
+; CHECK-SIMPLE-NOT: define {{.*}}addFive{{.*}}
+; CHECK-SIMPLE-NOT: define {{.*}}unusedFunc{{.*}}
+
+;CHECK-MULTIPLE-DEFS: error: Linking globals named {{.*}}bar_func1{{.*}} symbol multiply defined!
+
+; CHECK-DEVICE-LIB: define {{.*}}foo_func1{{.*}}
+; CHECK-DEVICE-LIB: define {{.*}}foo_func2{{.*}}
+; CHECK-DEVICE-LIB: define {{.*}}bar_func1{{.*}}
+; CHECK-DEVICE-LIB: define {{.*}}addFive{{.*}}
+; CHECK-DEVICE-LIB-NOT: define {{.*}}unusedFunc{{.*}}
diff --git a/clang/test/Driver/sycl-link-spirv-target.cpp b/clang/test/Driver/sycl-link-spirv-target.cpp
index 85566c67ea92b..7585ef8b14a59 100644
--- a/clang/test/Driver/sycl-link-spirv-target.cpp
+++ b/clang/test/Driver/sycl-link-spirv-target.cpp
@@ -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 -triple=spirv64 -Xlinker --library-path=/tmp -Xlinker --device-libs=lib1.bc,lib2.bc %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" "-triple=spirv64" "--library-path=/tmp" "--device-libs=lib1.bc,lib2.bc" "{{.*}}.bc" "-o" "a.out"
diff --git a/clang/tools/clang-sycl-linker/CMakeLists.txt b/clang/tools/clang-sycl-linker/CMakeLists.txt
index 5665ad7d7186e..382c0ca441940 100644
--- a/clang/tools/clang-sycl-linker/CMakeLists.txt
+++ b/clang/tools/clang-sycl-linker/CMakeLists.txt
@@ -1,6 +1,10 @@
set(LLVM_LINK_COMPONENTS
${LLVM_TARGETS_TO_BUILD}
BinaryFormat
+ BitWriter
+ Core
+ IRReader
+ Linker
Option
Object
TargetParser
diff --git a/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp b/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
index 2bcb3757d49d0..edde56486e7e8 100644
--- a/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
+++ b/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
@@ -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"
@@ -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());
@@ -191,7 +193,7 @@ Expected<SmallVector<std::string>> getInput(const ArgList &Args) {
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.
+ // This will be extended to support SPIR-V IR files.
if (Magic != file_magic::bitcode)
return createStringError("Unsupported file type");
BitcodeFiles.push_back(*Filename);
@@ -199,45 +201,23 @@ Expected<SmallVector<std::string>> getInput(const ArgList &Args) {
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))
- return std::move(Err);
- return Args.MakeArgString(*OutFileOrErr);
+/// 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.
+Expected<std::unique_ptr<Module>> getBitcodeModule(StringRef File,
+ LLVMContext &C) {
+ SMDiagnostic Err;
+
+ auto M = getLazyIRFileModule(File, Err, C);
+ if (M)
+ return std::move(M);
+ return createStringError("Unable to parse file");
}
-// This utility function is used to gather all SYCL device library files that
-// will be linked with input device files.
-// The list of files and its location are passed from driver.
+/// Gather all SYCL device library files that will be linked with input device
+/// files.
+/// The list of files and its location are passed from driver.
Expected<SmallVector<std::string>> getSYCLDeviceLibs(const ArgList &Args) {
SmallVector<std::string> DeviceLibFiles;
StringRef LibraryPath;
@@ -264,44 +244,84 @@ 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");
+
+ assert(InputFiles.size() && "No inputs to link");
+
+ LLVMContext C;
+ 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");
+ }
+ // 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();
+ // Link in SYCL device library files.
+ const llvm::Triple Triple(Args.getLastArgValue(OPT_triple_EQ));
+ for (auto &File : *SYCLDeviceLibFiles) {
+ auto LibMod = getBitcodeModule(File, C);
+ if (!LibMod)
+ return LibMod.takeError();
+ if ((*LibMod)->getTargetTriple() == Triple) {
+ unsigned Flags = Linker::Flags::LinkOnlyNeeded;
+ if (L.linkInModule(std::move(*LibMod), Flags))
+ return createStringError("Could not link IR");
+ }
+ }
+
+ // Dump linked output for testing.
+ if (Args.hasArg(OPT_print_linked_module))
+ outs() << *LinkerOutput;
// Create a new file to write the linked device file to.
- auto OutFileOrErr =
+ auto BitcodeOutput =
createTempFile(Args, sys::path::filename(OutputFile), "bc");
- if (!OutFileOrErr)
- return OutFileOrErr.takeError();
+ if (!BitcodeOutput)
+ return BitcodeOutput.takeError();
+
+ // 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);
+
+ if (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);
+ }
- 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;
+ return *BitcodeOutput;
}
/// Add any llvm-spirv option that relies on a specific Triple in addition
@@ -345,7 +365,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"
@@ -385,7 +405,7 @@ static Expected<StringRef> runLLVMToSPIRVTranslation(StringRef File,
SmallVector<StringRef, 8> CmdArgs;
CmdArgs.push_back(*LLVMToSPIRVProg);
- const llvm::Triple Triple(Args.getLastArgValue(OPT_triple));
+ const llvm::Triple Triple(Args.getLastArgValue(OPT_triple_EQ));
getSPIRVTransOpts(Args, CmdArgs, Triple);
StringRef LLVMToSPIRVOptions;
if (Arg *A = Args.getLastArg(OPT_llvm_spirv_options_EQ))
@@ -422,20 +442,19 @@ static Expected<StringRef> runLLVMToSPIRVTranslation(StringRef File,
return OutputFile;
}
+/// Performs the following steps:
+/// 1. Link input device code (user code and SYCL device library code).
+/// 2. Run SPIR-V code generation.
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(DeviceLinkedFil...
[truncated]
|
Can you please take a look? This is one of the many changes we are making to upstream SYCL. Thanks |
This PR has the following changes: Replace llvm-link with calls to linkInModule to link device files Add -print-linked-module option to dump linked module for testing Added a test to verify that linking is working as expected. We will eventually move to using thin LTO for linking device inputs. Thanks Signed-off-by: Arvind Sudarsanam <[email protected]>
Signed-off-by: Arvind Sudarsanam <[email protected]>
84b1894
to
9a97ac6
Compare
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 be able to just use LTO after the SPIR-V backend left experimental?
Hi @jhuber6 Thanks for the ping back. That is the eventual path for us. However, we would like to do it in stages. Replacing llvm-link tool with linkInModule call helps us to achieve one of our initial goals: To replace calls to external tools with library calls, wherever possible. Once we have an end-to-end device code linking available upstream, we should be able to replace linkInModule calls by using LTO. Thanks |
Signed-off-by: Arvind Sudarsanam <[email protected]>
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 straightforward enough. Hopefully in the future we can get this working with a normal LTO pipeline.
; CHECK-SIMPLE-NOT: define {{.*}}addFive{{.*}} | ||
; CHECK-SIMPLE-NOT: define {{.*}}unusedFunc{{.*}} | ||
|
||
;CHECK-MULTIPLE-DEFS: error: Linking globals named {{.*}}bar_func1{{.*}} symbol multiply defined! |
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.
;CHECK-MULTIPLE-DEFS: error: Linking globals named {{.*}}bar_func1{{.*}} symbol multiply defined! | |
; CHECK-MULTIPLE-DEFS: error: Linking globals named {{.*}}bar_func1{{.*}} symbol multiply defined! |
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.
Can I please add this change to my next PR? Just to avoid another round of testing? Thanks
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.
Can do it here...Thanks
@@ -345,7 +354,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" |
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.
Unrelated?
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.
This list will actually be removed in my next PR that replace llvm-spirv translator with SPIR-V backend codegen (I will submit that one right after merging this one). Just FYI, this change was required for testing with other downstream tools.
Thanks
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 will remove this from this PR. Thanks
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.
lgtm minus nit/minor comment!
|
||
auto M = getLazyIRFileModule(File, Err, C); | ||
if (M) | ||
return std::move(M); |
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.
nit: i don't think we need the std::move
here
// TODO: Support SPIR-V IR files. | ||
Expected<std::unique_ptr<Module>> getBitcodeModule(StringRef File, | ||
LLVMContext &C) { | ||
SMDiagnostic Err; |
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.
Do we need to check this error?
Signed-off-by: Arvind Sudarsanam <[email protected]>
Thanks so much @kazutakahirata |
I believe this broke one of our bots: https://lab.llvm.org/buildbot/#/builders/140/builds/20266
|
Probably needs a |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/13637 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/18/builds/13838 Here is the relevant piece of the build log for the reference
|
Sanitizer test should be fixed by change from @kazutakahirata (9586117) llvm-arm-ubuntu fail also should be fixed by this. Thanks |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/15276 Here is the relevant piece of the build log for the reference
|
This fixes error reported in post-commit testing of #133797 LOG: https://lab.llvm.org/buildbot/#/builders/140/builds/20266 Thanks Signed-off-by: Arvind Sudarsanam <[email protected]>
lldb-api :: lang/c/global_variables/TestGlobalVariables.py failure reported in https://lab.llvm.org/buildbot/#/builders/59/builds/15276 does not seem to be related to my change. Thanks |
This fixes error reported in post-commit testing of llvm/llvm-project#133797 LOG: https://lab.llvm.org/buildbot/#/builders/140/builds/20266 Thanks Signed-off-by: Arvind Sudarsanam <[email protected]>
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/10400 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/72/builds/9760 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/145/builds/6179 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/64/builds/2733 Here is the relevant piece of the build log for the reference
|
This PR has the following changes: Replace llvm-link with calls to linkInModule to link device files Add -print-linked-module option to dump linked module for testing Added a test to verify that linking is working as expected. We will eventually move to using thin LTO for linking device inputs. Thanks --------- Signed-off-by: Arvind Sudarsanam <[email protected]>
This fixes error reported in post-commit testing of llvm#133797 LOG: https://lab.llvm.org/buildbot/#/builders/140/builds/20266 Thanks Signed-off-by: Arvind Sudarsanam <[email protected]>
This PR has the following changes:
Replace llvm-link with calls to linkInModule to link device files Add -print-linked-module option to dump linked module for testing Added a test to verify that linking is working as expected. We will eventually move to using thin LTO for linking device inputs.
Thanks