[Clang][SYCL] Introduce clang-sycl-link-wrapper to link SYCL offloading device code #1

asudarsa · 2024-10-07T14:45:29Z

This PR is one of the many PRs in the SYCL upstreaming effort focusing on device code linking during the SYCL offload compilation process. RFC: https://discourse.llvm.org/t/rfc-offloading-design-for-sycl-offload-kind-and-spir-targets/74088

In this PR, we introduce a new tool that will be used to perform device code linking for SYCL offload kind. It accepts SYCL device objects in LLVM IR bitcode format and will generate a fully linked device object that can then be wrapped and linked into the host object.

A primary use case for this tool is to perform device code linking for objects with SYCL offload kind inside the clang-linker-wrapper. It can also be invoked via clang driver as follows:

clang --target=spirv64 --sycl-link input.bc

Device code linking for SYCL offloading kind has a number of known quirks that makes it difficult to use in a unified offloading setting. Two of the primary issues are:

Several finalization steps are required to be run on the fully-linked LLVM IR bitcode to gaurantee conformance to SYCL standards. This step is unique to SYCL offloading compilation flow.
SPIR-V LLVM Translator tool is an extenal tool and hence SPIR-V IR code generation cannot be done as part of LTO. This limitation will be lifted once SPIR-V backend is available as a viable LLVM backend.

Hence, we introduce this new tool to provide a clean wrapper to perform SYCL device linking.

Thanks

…ng device code This PR is one of the many PRs in the SYCL upstreaming effort focusing on device code linking during the SYCL offload compilation process. RFC: https://discourse.llvm.org/t/rfc-offloading-design-for-sycl-offload-kind-and-spir-targets/74088 In this PR, we introduce a new tool that will be used to perform device code linking for SYCL offload kind. It accepts SYCL device objects in LLVM IR bitcode format and will generate a fully linked device object that can then be wrapped and linked into the host object. A primary use case for this tool is to perform device code linking for objects with SYCL offload kind inside the clang-linker-wrapper. It can also be invoked via clang driver as follows: `clang --target=spirv64 --sycl-link input.bc` Device code linking for SYCL offloading kind has a number of known quirks that makes it difficult to use in a unified offloading setting. Two of the primary issues are: 1. Several finalization steps are required to be run on the fully-linked LLVM IR bitcode to gaurantee conformance to SYCL standards. This step is unique to SYCL offloading compilation flow. 2. SPIR-V LLVM Translator tool is an extenal tool and hence SPIR-V IR code generation cannot be done as part of LTO. This limitation will be lifted once SPIR-V backend is available as a viable LLVM backend. Hence, we introduce this new tool to provide a clean wrapper to perform SYCL device linking. Thanks Signed-off-by: Arvind Sudarsanam <[email protected]>

sarnex

looks great, thanks! just some comments below.

sarnex · 2024-10-07T16:15:32Z

clang/docs/ClangSYCLLinkWrapper.rst

+============
+
+This tool works as a wrapper around the SYCL device code linking process.
+Purpose of this wrapper is to provide an interface to link SYCL device bitcode


Suggested change

Purpose of this wrapper is to provide an interface to link SYCL device bitcode

The purpose of this wrapper is to provide an interface to link SYCL device bitcode

sarnex · 2024-10-07T16:15:53Z

clang/docs/ClangSYCLLinkWrapper.rst

+in LLVM IR format, run some SYCL-specific finalization steps and then use the
+SPIR-V LLVM Translator tool to produce the final output.
+
+Device code linking for SYCL offloading kind has a number of known quirks that


Suggested change

Device code linking for SYCL offloading kind has a number of known quirks that

Device code linking for SYCL offloading has a number of known quirks that

sarnex · 2024-10-07T16:16:22Z

clang/docs/ClangSYCLLinkWrapper.rst

+makes it difficult to use in a unified offloading setting. Two of the primary
+issues are:
+1. Several finalization steps are required to be run on the fully-linked LLVM
+IR bitcode to gaurantee conformance to SYCL standards. This step is unique to


Suggested change

IR bitcode to gaurantee conformance to SYCL standards. This step is unique to

IR bitcode to guarantee conformance to SYCL standards. This step is unique to

sarnex · 2024-10-07T16:16:32Z

clang/docs/ClangSYCLLinkWrapper.rst

+issues are:
+1. Several finalization steps are required to be run on the fully-linked LLVM
+IR bitcode to gaurantee conformance to SYCL standards. This step is unique to
+SYCL offloading compilation flow.


Suggested change

SYCL offloading compilation flow.

the SYCL offloading compilation flow.

sarnex · 2024-10-07T16:16:55Z

clang/docs/ClangSYCLLinkWrapper.rst

+1. Several finalization steps are required to be run on the fully-linked LLVM
+IR bitcode to gaurantee conformance to SYCL standards. This step is unique to
+SYCL offloading compilation flow.
+2. SPIR-V LLVM Translator tool is an extenal tool and hence SPIR-V IR code


Suggested change

2. SPIR-V LLVM Translator tool is an extenal tool and hence SPIR-V IR code

2. SPIR-V LLVM Translator tool is an external tool and hence SPIR-V IR code

sarnex · 2024-10-07T20:41:41Z

clang/tools/clang-sycl-link-wrapper/ClangSYCLLinkWrapper.cpp

+const SmallVector<std::string> SYCLDeviceLibNames = {
+    "libsycl-crt.bc",
+    "libsycl-complex.bc",
+    "libsycl-complex-fp64.bc",


i forget, are there plans to have the driver own this list?

sarnex · 2024-10-07T20:42:27Z

clang/tools/clang-sycl-link-wrapper/ClangSYCLLinkWrapper.cpp

+  for (auto &DeviceLibName : SYCLDeviceLibNames) {
+    std::optional<std::string> Filename =
+        findFile(LibraryPath, /*Root=*/"", DeviceLibName);
+    if (Filename)


is nullopt return valid? should it be an error?

sarnex · 2024-10-07T20:44:42Z

clang/tools/clang-sycl-link-wrapper/ClangSYCLLinkWrapper.cpp

+
+    StringRef Sep = llvm::sys::path::get_separator();
+    StringRef Path = OutputFile;
+    StringRef Filename = Path.rsplit(Sep).second;


I see the llvm::sys::path namespace has a filename function to extract the filename from the path, can we use that here?

sarnex · 2024-10-07T20:45:31Z

clang/tools/clang-sycl-link-wrapper/ClangSYCLLinkWrapper.cpp

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


do we want to note in a comment or something that we will need to add processing of the linked module (sycl-post-link) later?

sarnex · 2024-10-07T20:47:16Z

clang/tools/clang-sycl-link-wrapper/ClangSYCLLinkWrapper.cpp

+    reportError(std::move(Err));
+
+  // Remove the temporary files created.
+  if (!Args.hasArg(OPT_save_temps))


if there's an error, i think we won't remove the temp files even if --save-temps is not passed, right? reportError is noreturn, so we probably need something that will always run

bader · 2024-10-07T23:27:12Z

A primary use case for this tool is to perform device code linking for objects with SYCL offload kind inside the clang-linker-wrapper. It can also be invoked via clang driver as follows:

clang --target=spirv64 --sycl-link input.bc

Device code linking for SYCL offloading kind has a number of known quirks that makes it difficult to use in a unified offloading setting. Two of the primary issues are:

Several finalization steps are required to be run on the fully-linked LLVM IR bitcode to gaurantee conformance to SYCL standards. This step is unique to SYCL offloading compilation flow.

I think it might be easier to communicate SYCL runtime requirements by adding SYCL operating system type or environment type to the target triple.

clang -target=spirv64-intel-sycl input.bc # OS type
clang -target=spirv64-intel--sycl input.bc # Environment type

E.g. CUDA uses CUDA OS type to select the right interface with the driver. See https://llvm.org/docs/NVPTXUsage.html#triples

One concern is that Codeplay is already using cuda OS type when we compiler SYCL for nvptx target. @Naghasan, what do you think about the idea of having SYCL specific OS type in the target triple?

bader · 2024-10-07T23:28:15Z

clang/docs/ClangSYCLLinkWrapper.rst

+====================
+Clang SYCL link Wrapper
+====================


Suggested change

====================

Clang SYCL link Wrapper

====================

=======================

Clang SYCL link Wrapper

=======================

bader · 2024-10-07T23:33:03Z

clang/docs/ClangSYCLLinkWrapper.rst

+============
+
+This tool works as a wrapper around the SYCL device code linking process.
+Purpose of this wrapper is to provide an interface to link SYCL device bitcode


According to my understanding, the purpose of the tool is to provide standard linker interface and be able to link any object file formats 'compile' step is able to produce for SYCL offload mode. So, I would expect this tool to handle linking of fat objects with device code in other IR like SPIR-V as well as native binary format.

bader · 2024-10-07T23:35:29Z

clang/docs/ClangSYCLLinkWrapper.rst

+=====
+
+This tool can be used with the following options. Several of these options will
+be passed down to downstrea tools like 'llvm-link', 'llvm-spirv', etc.


Suggested change

be passed down to downstrea tools like 'llvm-link', 'llvm-spirv', etc.

be passed down to downstrea tools like 'clang', 'llvm-spirv', etc.

We should use clang instead of llvm-link to link llvm bitcode files.

bader · 2024-10-07T23:37:24Z

clang/docs/ClangSYCLLinkWrapper.rst

+.. code-block:: console
+
+  OVERVIEW: A utility that wraps around the SYCl device code linking process.
+  This enables linking and code generation for SPIR-V JIT targets.


What about AOT targets?

bader · 2024-10-07T23:40:31Z

clang/docs/ClangSYCLLinkWrapper.rst

+=======
+
+This tool is intended to be invoked when targeting the SPIR-V toolchain.
+When --sycl-link option is passed, clang driver will invoke the linking job of


Please, specify that --sycl-link is the clang driver option, not clang-sycl-link-wrapper option.

bader · 2024-10-07T23:46:46Z

clang/tools/clang-sycl-link-wrapper/SYCLLinkOpts.td

@@ -0,0 +1,50 @@
+// We try to create options similar to lld's.  That way, options passed to clang
+// -Xoffload-linker can be the same whether offloading to nvptx or amdgpu.


Copy-paste from another source? I expect this comment reference spir-v instead of nvptx and amdgpu.

bader · 2024-10-07T23:49:33Z

clang/tools/clang-sycl-link-wrapper/SYCLLinkOpts.td

+
+def sycl_dump_device_code_EQ : Joined<["--", "-"], "sycl-dump-device-code=">,
+  Flags<[WrapperOnlyOption]>,
+  HelpText<"Path to the folder where the tool dumps SPIR-V device code. Other formats aren't dumped.">;


We should either change sycl to spirv in the option name of vice versa in the description. Otherwise, it looks a bit messy.

bader · 2024-10-07T23:50:19Z

clang/tools/clang-sycl-link-wrapper/SYCLLinkOpts.td

+def sycl_is_windows_msvc_env : Flag<["--", "-"], "sycl-is-windows-msvc-env">,
+  Flags<[WrapperOnlyOption, HelpHidden]>;
+
+// Special option to pass in llvm-spirv options


Suggested change

// Special option to pass in llvm-spirv options

// Options to pass to llvm-spirv tool

bader · 2024-10-07T23:51:36Z

clang/tools/clang-sycl-link-wrapper/ClangSYCLLinkWrapper.cpp

+using namespace llvm::opt;
+using namespace llvm::object;
+
+/// Ssave intermediary results.


Suggested change

/// Ssave intermediary results.

/// Save intermediary results.

bader · 2024-10-07T23:52:52Z

clang/tools/clang-sycl-link-wrapper/ClangSYCLLinkWrapper.cpp

+static StringRef OutputFile;
+
+/// Directory to dump SPIR-V IR if requested by user.
+SmallString<128> SPIRVDumpDir;


Suggested change

SmallString<128> SPIRVDumpDir;

static SmallString<128> SPIRVDumpDir;

asudarsa changed the title ~~[Clang][SYCL] Introduce clang-sycl-link-wrapper to link SYCL offloadi…~~ [Clang][SYCL] Introduce clang-sycl-link-wrapper to link SYCL offloading device code Oct 7, 2024

sarnex reviewed Oct 7, 2024

View reviewed changes

bader reviewed Oct 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang][SYCL] Introduce clang-sycl-link-wrapper to link SYCL offloading device code #1

[Clang][SYCL] Introduce clang-sycl-link-wrapper to link SYCL offloading device code #1

asudarsa commented Oct 7, 2024 •

edited

Loading

sarnex left a comment

sarnex Oct 7, 2024

sarnex Oct 7, 2024

sarnex Oct 7, 2024

sarnex Oct 7, 2024

sarnex Oct 7, 2024

sarnex Oct 7, 2024

sarnex Oct 7, 2024

sarnex Oct 7, 2024

sarnex Oct 7, 2024

sarnex Oct 7, 2024

bader commented Oct 7, 2024

bader Oct 7, 2024

bader Oct 7, 2024

bader Oct 7, 2024

bader Oct 7, 2024

bader Oct 7, 2024

bader Oct 7, 2024

bader Oct 7, 2024

bader Oct 7, 2024

bader Oct 7, 2024

bader Oct 7, 2024

	Purpose of this wrapper is to provide an interface to link SYCL device bitcode
	The purpose of this wrapper is to provide an interface to link SYCL device bitcode

	IR bitcode to gaurantee conformance to SYCL standards. This step is unique to
	IR bitcode to guarantee conformance to SYCL standards. This step is unique to

	SYCL offloading compilation flow.
	the SYCL offloading compilation flow.

	2. SPIR-V LLVM Translator tool is an extenal tool and hence SPIR-V IR code
	2. SPIR-V LLVM Translator tool is an external tool and hence SPIR-V IR code

	be passed down to downstrea tools like 'llvm-link', 'llvm-spirv', etc.
	be passed down to downstrea tools like 'clang', 'llvm-spirv', etc.

		@@ -0,0 +1,50 @@
		// We try to create options similar to lld's. That way, options passed to clang
		// -Xoffload-linker can be the same whether offloading to nvptx or amdgpu.

	// Special option to pass in llvm-spirv options
	// Options to pass to llvm-spirv tool

	/// Ssave intermediary results.
	/// Save intermediary results.

	SmallString<128> SPIRVDumpDir;
	static SmallString<128> SPIRVDumpDir;

[Clang][SYCL] Introduce clang-sycl-link-wrapper to link SYCL offloading device code #1

Are you sure you want to change the base?

[Clang][SYCL] Introduce clang-sycl-link-wrapper to link SYCL offloading device code #1

Conversation

asudarsa commented Oct 7, 2024 • edited Loading

sarnex left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bader commented Oct 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asudarsa commented Oct 7, 2024 •

edited

Loading