Skip to content

[NewOffloadModel][SYCL DeviceLib] Generate SYCL device library objects using new offload model #13579

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
May 7, 2024
Merged

[NewOffloadModel][SYCL DeviceLib] Generate SYCL device library objects using new offload model #13579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
16952fc
[NewOffloadModel][SYCL DeviceLib] Generate SYCL device library object…
asudarsa Apr 25, 2024
ee4227c
Remove --offload-new-driver when building SYCL device libs for nvidia…
asudarsa Apr 27, 2024
f1caa37
Fix test failures and reintroduce --offload-new-driver for nvptx
asudarsa May 1, 2024
c43cef5
Add dummy device arch for nvidia device lib gen
asudarsa May 2, 2024
41bc9e9
Adding updated copies of device library files
asudarsa May 2, 2024
ff0fd35
Fix clang format issue
asudarsa May 2, 2024
07144b5
Fix some merge issues
asudarsa May 2, 2024
3700991
Add windows test and address review comments
asudarsa May 6, 2024
66de5b9
Fix incorrect host triple
asudarsa May 6, 2024
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
10 changes: 9 additions & 1 deletion clang/lib/Driver/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3683,8 +3683,11 @@ getLinkerArgs(Compilation &C, DerivedArgList &Args, bool IncludeObj = false) {
static bool IsSYCLDeviceLibObj(std::string ObjFilePath, bool isMSVCEnv) {
StringRef ObjFileName = llvm::sys::path::filename(ObjFilePath);
StringRef ObjSuffix = isMSVCEnv ? ".obj" : ".o";
StringRef NewObjSuffix = isMSVCEnv ? ".new.obj" : ".new.o";
Copy link
Contributor

@maksimsab maksimsab May 3, 2024

Choose a reason for hiding this comment

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

It looks like we miss tests with ".new.obj".

Copy link
Contributor Author

@asudarsa asudarsa May 3, 2024

Choose a reason for hiding this comment

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

let me add them. Thanks

Copy link
Contributor Author

@asudarsa asudarsa May 6, 2024

Choose a reason for hiding this comment

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

Adding a new test in upcoming commit. Thanks

bool Ret =
(ObjFileName.starts_with("libsycl-") && ObjFileName.ends_with(ObjSuffix))
(ObjFileName.starts_with("libsycl-") &&
ObjFileName.ends_with(ObjSuffix) &&
!ObjFileName.ends_with(NewObjSuffix)) // Avoid new-offload-driver objs
? true
: false;
return Ret;
Expand Down Expand Up @@ -7877,6 +7880,11 @@ Action *Driver::BuildOffloadingActions(Compilation &C,
break;
}

// Backend/Assemble actions are not used for the SYCL device side
Copy link
Contributor Author

@asudarsa asudarsa May 2, 2024

Choose a reason for hiding this comment

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

Change (7883-7887) provided by @mdtoguchi. Thanks Mike. For SYCL offloading, we want -c compilation to NOT invoke backend tools. For new offloading model, we see that the backend tools are being called for -c compilation. This change prevents the backend tools from being invoked for -c compilation.

if (Kind == Action::OFK_SYCL &&
(Phase == phases::Backend || Phase == phases::Assemble))
continue;

auto TCAndArch = TCAndArchs.begin();
for (Action *&A : DeviceActions) {
if (A->getType() == types::TY_Nothing)
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11049,7 +11049,10 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
SYCLInstallationDetector SYCLInstallation(D);
SYCLInstallation.getSYCLDeviceLibPath(LibLocCandidates);
SmallString<128> LibName("libsycl-crt");
StringRef LibSuffix = TheTriple.isWindowsMSVCEnvironment() ? ".obj" : ".o";
bool IsNewOffload = D.getUseNewOffloadingDriver();
StringRef LibSuffix = TheTriple.isWindowsMSVCEnvironment()
? (IsNewOffload ? ".new.obj" : ".obj")
: (IsNewOffload ? ".new.o" : ".o");
llvm::sys::path::replace_extension(LibName, LibSuffix);
for (const auto &LibLoc : LibLocCandidates) {
SmallString<128> FullLibName(LibLoc);
Expand Down
21 changes: 16 additions & 5 deletions clang/lib/Driver/ToolChains/SYCL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,16 @@ SYCL::getDeviceLibraries(const Compilation &C, const llvm::Triple &TargetTriple,
const SYCLDeviceLibsList SYCLDeviceSanitizerLibs = {
{"libsycl-sanitizer", "internal"}};
#endif
bool IsWindowsMSVCEnv =
C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment();
bool IsNewOffload = C.getDriver().getUseNewOffloadingDriver();
StringRef LibSuffix = ".bc";
if (TargetTriple.isNVPTX())
// For NVidia, we are unbundling objects.
LibSuffix = C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment()
? ".obj"
: ".o";
LibSuffix = IsWindowsMSVCEnv ? ".obj" : ".o";
if (IsNewOffload)
// For new offload model, we use packaged .bc files.
LibSuffix = IsWindowsMSVCEnv ? ".new.obj" : ".new.o";
auto addLibraries = [&](const SYCLDeviceLibsList &LibsList) {
for (const DeviceLibOptInfo &Lib : LibsList) {
if (!DeviceLibLinkInfo[Lib.DeviceLibOption])
Expand Down Expand Up @@ -441,19 +445,26 @@ const char *SYCL::Linker::constructLLVMLinkCommand(
C.getDriver().IsCLMode())
LibPostfix = ".obj";
}
StringRef NewLibPostfix = ".new.o";
if (HostTC->getTriple().isWindowsMSVCEnvironment() &&
C.getDriver().IsCLMode())
NewLibPostfix = ".new.obj";
std::string FileName = this->getToolChain().getInputFilename(II);
StringRef InputFilename = llvm::sys::path::filename(FileName);
if (IsNVPTX || IsSYCLNativeCPU) {
// Linking SYCL Device libs requires libclc as well as libdevice
if ((InputFilename.find("libspirv") != InputFilename.npos ||
InputFilename.find("libdevice") != InputFilename.npos))
return true;
if (IsNVPTX)
if (IsNVPTX) {
LibPostfix = ".cubin";
NewLibPostfix = ".new.cubin";
Copy link
Contributor

@maksimsab maksimsab May 3, 2024

Choose a reason for hiding this comment

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

It looks like we miss tests with ".new.cubin".

Copy link
Contributor Author

@asudarsa asudarsa May 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

.cubin are intermediate filenames that are created in the old offloading driver flow. So, we cannot add tests for this. I will discuss with relevant experts on why we need this renaming.
Thanks

}
}
StringRef LibSyclPrefix("libsycl-");
if (!InputFilename.starts_with(LibSyclPrefix) ||
!InputFilename.ends_with(LibPostfix))
!InputFilename.ends_with(LibPostfix) ||
InputFilename.ends_with(NewLibPostfix))
return false;
// Skip the prefix "libsycl-"
std::string PureLibName =
Expand Down
Binary file added clang/test/Driver/Inputs/libsycl-complex.new.o
View file
Open in desktop
Binary file not shown.
Binary file added clang/test/Driver/Inputs/libsycl-complex.new.obj
View file
Open in desktop
Binary file not shown.
Binary file added clang/test/Driver/Inputs/libsycl-crt.new.o
View file
Open in desktop
Binary file not shown.
Binary file added clang/test/Driver/Inputs/libsycl-crt.new.obj
View file
Open in desktop
Binary file not shown.
12 changes: 12 additions & 0 deletions clang/test/Driver/linker-wrapper-sycl-win.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// REQUIRES: system-windows

/// Check for list of commands for standalone clang-linker-wrapper run for sycl
// RUN: clang-linker-wrapper -sycl-device-library-location=%S/Inputs -sycl-device-libraries=libsycl-crt.new.obj,libsycl-complex.new.obj -sycl-post-link-options="SYCL_POST_LINK_OPTIONS" -llvm-spirv-options="LLVM_SPIRV_OPTIONS" "--host-triple=x86_64-pc-windows-msvc" "--triple=spir64" "--linker-path=/usr/bin/ld" "--" HOST_LINKER_FLAGS "-dynamic-linker" HOST_DYN_LIB "-o" "a.out" HOST_LIB_PATH HOST_STAT_LIB %S/Inputs/test-sycl.o --dry-run 2>&1 | FileCheck -check-prefix=CHK-CMDS %s
// CHK-CMDS: "{{.*}}spirv-to-ir-wrapper.exe" {{.*}} -o [[FIRSTLLVMLINKIN:.*]].bc --llvm-spirv-opts=--spirv-preserve-auxdata --llvm-spirv-opts=--spirv-target-env=SPV-IR --llvm-spirv-opts=--spirv-builtin-format=global
// CHK-CMDS-NEXT: "{{.*}}llvm-link.exe" [[FIRSTLLVMLINKIN:.*]].bc -o [[FIRSTLLVMLINKOUT:.*]].bc --suppress-warnings
// CHK-CMDS-NEXT: "{{.*}}llvm-link.exe" -only-needed [[FIRSTLLVMLINKOUT]].bc {{.*}}.bc {{.*}}.bc -o [[SECONDLLVMLINKOUT:.*]].bc --suppress-warnings
// CHK-CMDS-NEXT: "{{.*}}sycl-post-link.exe" SYCL_POST_LINK_OPTIONS -o [[SYCLPOSTLINKOUT:.*]].table [[SECONDLLVMLINKOUT]].bc
// LLVM-SPIRV is not called in dry-run
// CHK-CMDS-NEXT: offload-wrapper: input: [[LLVMSPIRVOUT:.*]].table, output: [[WRAPPEROUT:.*]].bc
// CHK-CMDS-NEXT: "{{.*}}llc.exe" -filetype=obj -o [[LLCOUT:.*]].o [[WRAPPEROUT]].bc
// CHK-CMDS-NEXT: "{{.*}}/ld" -- HOST_LINKER_FLAGS -dynamic-linker HOST_DYN_LIB -o a.out [[LLCOUT]].o HOST_LIB_PATH HOST_STAT_LIB {{.*}}test-sycl.o
Copy link
Contributor

@maksimsab maksimsab May 7, 2024

Choose a reason for hiding this comment

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

nit.
It seems for me that there is no ld linker present in the Windows environment. There is different linker specific to windows - link.exe. It is not relevant to the current PR.

9 changes: 4 additions & 5 deletions clang/test/Driver/linker-wrapper-sycl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
// REQUIRES: system-linux

/// Check for list of commands for standalone clang-linker-wrapper run for sycl
// RUN: clang-linker-wrapper -sycl-device-library-location=%S/Inputs -sycl-device-libraries=libsycl-crt.o,libsycl-complex.o -sycl-post-link-options="SYCL_POST_LINK_OPTIONS" -llvm-spirv-options="LLVM_SPIRV_OPTIONS" "--host-triple=x86_64-unknown-linux-gnu" "--triple=spir64" "--linker-path=/usr/bin/ld" "--" HOST_LINKER_FLAGS "-dynamic-linker" HOST_DYN_LIB "-o" "a.out" HOST_LIB_PATH HOST_STAT_LIB %S/Inputs/test-sycl.o --dry-run 2>&1 | FileCheck -check-prefix=CHK-CMDS %s
// CHK-CMDS: "{{.*}}llvm-link" [[INPUT:.*]].bc -o [[FIRSTLLVMLINKOUT:.*]].bc --suppress-warnings
// CHK-CMDS-NEXT: "{{.*}}clang-offload-bundler" -type=o -targets=sycl-spir64-unknown-unknown -input={{.*}}libsycl-crt.o -output=[[FIRSTUNBUNDLEDLIB:.*]].bc -unbundle -allow-missing-bundles
// CHK-CMDS-NEXT: "{{.*}}clang-offload-bundler" -type=o -targets=sycl-spir64-unknown-unknown -input={{.*}}libsycl-complex.o -output=[[SECONDUNBUNDLEDLIB:.*]].bc -unbundle -allow-missing-bundles
// CHK-CMDS-NEXT: "{{.*}}llvm-link" -only-needed [[FIRSTLLVMLINKOUT]].bc [[FIRSTUNBUNDLEDLIB]].bc [[SECONDUNBUNDLEDLIB]].bc -o [[SECONDLLVMLINKOUT:.*]].bc --suppress-warnings
// RUN: clang-linker-wrapper -sycl-device-library-location=%S/Inputs -sycl-device-libraries=libsycl-crt.new.o,libsycl-complex.new.o -sycl-post-link-options="SYCL_POST_LINK_OPTIONS" -llvm-spirv-options="LLVM_SPIRV_OPTIONS" "--host-triple=x86_64-unknown-linux-gnu" "--triple=spir64" "--linker-path=/usr/bin/ld" "--" HOST_LINKER_FLAGS "-dynamic-linker" HOST_DYN_LIB "-o" "a.out" HOST_LIB_PATH HOST_STAT_LIB %S/Inputs/test-sycl.o --dry-run 2>&1 | FileCheck -check-prefix=CHK-CMDS %s
// CHK-CMDS: "{{.*}}spirv-to-ir-wrapper" {{.*}} -o [[FIRSTLLVMLINKIN:.*]].bc --llvm-spirv-opts=--spirv-preserve-auxdata --llvm-spirv-opts=--spirv-target-env=SPV-IR --llvm-spirv-opts=--spirv-builtin-format=global
// CHK-CMDS-NEXT: "{{.*}}llvm-link" [[FIRSTLLVMLINKIN:.*]].bc -o [[FIRSTLLVMLINKOUT:.*]].bc --suppress-warnings
// CHK-CMDS-NEXT: "{{.*}}llvm-link" -only-needed [[FIRSTLLVMLINKOUT]].bc {{.*}}.bc {{.*}}.bc -o [[SECONDLLVMLINKOUT:.*]].bc --suppress-warnings
// CHK-CMDS-NEXT: "{{.*}}sycl-post-link" SYCL_POST_LINK_OPTIONS -o [[SYCLPOSTLINKOUT:.*]].table [[SECONDLLVMLINKOUT]].bc
// LLVM-SPIRV is not called in dry-run
// CHK-CMDS-NEXT: offload-wrapper: input: [[LLVMSPIRVOUT:.*]].table, output: [[WRAPPEROUT:.*]].bc
Expand Down
27 changes: 11 additions & 16 deletions clang/test/Driver/sycl-offload-new-driver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: system-linux

/// Verify --offload-new-driver option phases
// RUN: %clang --target=x86_64-unknown-linux-gnu -fsycl -fsycl-targets=nvptx64-nvidia-cuda,spir64 --offload-new-driver -ccc-print-phases %s 2>&1 \
// RUN: | FileCheck -check-prefix=OFFLOAD-NEW-DRIVER %s
Expand All @@ -10,20 +9,16 @@
// OFFLOAD-NEW-DRIVER: 4: input, "[[INPUT]]", c++, (device-sycl)
// OFFLOAD-NEW-DRIVER: 5: preprocessor, {4}, c++-cpp-output, (device-sycl)
// OFFLOAD-NEW-DRIVER: 6: compiler, {5}, ir, (device-sycl)
// OFFLOAD-NEW-DRIVER: 7: backend, {6}, assembler, (device-sycl)
// OFFLOAD-NEW-DRIVER: 8: assembler, {7}, object, (device-sycl)
// OFFLOAD-NEW-DRIVER: 9: offload, "device-sycl (nvptx64-nvidia-cuda)" {8}, object
// OFFLOAD-NEW-DRIVER: 10: input, "[[INPUT]]", c++, (device-sycl)
// OFFLOAD-NEW-DRIVER: 11: preprocessor, {10}, c++-cpp-output, (device-sycl)
// OFFLOAD-NEW-DRIVER: 12: compiler, {11}, ir, (device-sycl)
// OFFLOAD-NEW-DRIVER: 13: backend, {12}, assembler, (device-sycl)
// OFFLOAD-NEW-DRIVER: 14: assembler, {13}, object, (device-sycl)
// OFFLOAD-NEW-DRIVER: 15: offload, "device-sycl (spir64-unknown-unknown)" {14}, object
// OFFLOAD-NEW-DRIVER: 16: clang-offload-packager, {9, 15}, image, (device-sycl)
// OFFLOAD-NEW-DRIVER: 17: offload, "host-sycl (x86_64-unknown-linux-gnu)" {3}, "device-sycl (x86_64-unknown-linux-gnu)" {16}, ir
// OFFLOAD-NEW-DRIVER: 18: backend, {17}, assembler, (host-sycl)
// OFFLOAD-NEW-DRIVER: 19: assembler, {18}, object, (host-sycl)
// OFFLOAD-NEW-DRIVER: 20: clang-linker-wrapper, {19}, image, (host-sycl)
// OFFLOAD-NEW-DRIVER: 7: offload, "device-sycl (nvptx64-nvidia-cuda)" {6}, ir
// OFFLOAD-NEW-DRIVER: 8: input, "[[INPUT]]", c++, (device-sycl)
// OFFLOAD-NEW-DRIVER: 9: preprocessor, {8}, c++-cpp-output, (device-sycl)
// OFFLOAD-NEW-DRIVER: 10: compiler, {9}, ir, (device-sycl)
// OFFLOAD-NEW-DRIVER: 11: offload, "device-sycl (spir64-unknown-unknown)" {10}, ir
// OFFLOAD-NEW-DRIVER: 12: clang-offload-packager, {7, 11}, image, (device-sycl)
// OFFLOAD-NEW-DRIVER: 13: offload, "host-sycl (x86_64-unknown-linux-gnu)" {3}, "device-sycl (x86_64-unknown-linux-gnu)" {12}, ir
// OFFLOAD-NEW-DRIVER: 14: backend, {13}, assembler, (host-sycl)
// OFFLOAD-NEW-DRIVER: 15: assembler, {14}, object, (host-sycl)
// OFFLOAD-NEW-DRIVER: 16: clang-linker-wrapper, {15}, image, (host-sycl)

/// Check the toolflow for SYCL compilation using new offload model
// RUN: %clangxx -### --target=x86_64-unknown-linux-gnu -fsycl -fsycl-targets=spir64 --offload-new-driver %s 2>&1 | FileCheck -check-prefix=CHK-FLOW %s
Expand All @@ -38,7 +33,7 @@
// RUN: --sysroot=%S/Inputs/SYCL -### %s 2>&1 \
// RUN: | FileCheck -check-prefix WRAPPER_OPTIONS %s
// WRAPPER_OPTIONS: clang-linker-wrapper{{.*}} "--triple=spir64"
// WRAPPER_OPTIONS-SAME: "-sycl-device-libraries=libsycl-crt.bc,libsycl-complex.bc,libsycl-complex-fp64.bc,libsycl-cmath.bc,libsycl-cmath-fp64.bc,libsycl-imf.bc,libsycl-imf-fp64.bc,libsycl-imf-bf16.bc,libsycl-itt-user-wrappers.bc,libsycl-itt-compiler-wrappers.bc,libsycl-itt-stubs.bc"
// WRAPPER_OPTIONS-SAME: "-sycl-device-libraries=libsycl-crt.new.o,libsycl-complex.new.o,libsycl-complex-fp64.new.o,libsycl-cmath.new.o,libsycl-cmath-fp64.new.o,libsycl-imf.new.o,libsycl-imf-fp64.new.o,libsycl-imf-bf16.new.o,libsycl-itt-user-wrappers.new.o,libsycl-itt-compiler-wrappers.new.o,libsycl-itt-stubs.new.o"
Copy link
Contributor

@maksimsab maksimsab May 3, 2024

Choose a reason for hiding this comment

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

Can you help me to understand this change?

Copy link
Contributor

@sarnex sarnex May 6, 2024

Choose a reason for hiding this comment

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

Yeah I have a similar question. Arvind had mentioned supporting raw BC files in the linker wrapper was difficult so we were using fat objects, but in this test we are giving it raw BC files. Is that just a test thing and it wouldn't have actually worked with raw BC files and always required fat objects?

Copy link
Contributor Author

@asudarsa asudarsa May 6, 2024

Choose a reason for hiding this comment

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

the older version with .bc files was a recent change introduced by @mdtoguchi when he added change to replace bundled fat devicelib object with .bc files. .bc files can be easily consumed in the old offloading model. However, .bc files cannot be consumed by the current flow in clang-linker-wrapper. It requires packaged fat object files. All .new.o files are devicelib bc files packaged and thus can be consumed easily by the clang-linker-wrapper.

// WRAPPER_OPTIONS-SAME: "-sycl-device-library-location={{.*}}/lib"

// RUN: %clangxx --target=x86_64-unknown-linux-gnu -fsycl --offload-new-driver \
Expand Down
Loading