[SYCL][Docs] Add SYCLBIN feature and format design document

[steffenlarsen]

This commit adds a design document detailing the SYCLBIN binary format for representing SYCL device kernel binaries to be loaded dynamically at runtime. Additionally, the design document details how this is to be handled by the SYCL runtime, driver and clang tooling.

As the design of SYCLBIN files relies heavily on the property sets, this PR also adds documentation to the existing property set functionality.

[bader]

SYCL design documentation predominantly uses Markdown format. Please, convert the document to Markdown format.

[steffenlarsen]

SYCL design documentation predominantly uses Markdown format. Please, convert the document to Markdown format.

Apologies! Old habits die hard. It should be good now.

[AlexeySachkov]

If we are talking about a specific CUDA property, is it still IR module? I suppose that I don't understand well enough what is PTX and what is its place in a toolchain.

Target-specific for me means "native", i.e. as if PTX is incorrectly assumed as IR module. Also, all IR modules are expected to share the same properties within an abstract module, right? If so, then maybe we should propagate that property up to the abstract module level and have PTX modules compiled for different SM versions as separate abstract modules?

[steffenlarsen]

If we are talking about a specific CUDA property, is it still IR module?

Because of the forward-compatibility of SM archs, my understanding is that we want PTX to be considered an IR type.

Target-specific for me means "native", i.e. as if PTX is incorrectly assumed as IR module. Also, all IR modules are expected to share the same properties within an abstract module, right? If so, then maybe we should propagate that property up to the abstract module level and have PTX modules compiled for different SM versions as separate abstract modules?

If we were to put the SM architecture information at abstract module level, I don't see how an abstract module would ever have more than one IR module and more than one native device code image. Granted, having the exact same properties is somewhat rare, but I would expect it to be the case if the user was to compile for multiple SM versions.

[gmlueck]

I agree that it seems like we would probably want to annotate the IR module with the CUDA virtual architecture in the case when the IR is PTX. I was thinking that we would use the IR-level metadata for stuff like this.

[steffenlarsen]

I've added a "target" field to the IR module metadata which contains the value of -fsycl-targets used for the given module, similar to "arch" in native device code images, with the exception that this key may be missing from the metadata in the case that the option wasn't specified, in which case the IR type should be enough to infer from.

[AlexeySachkov]

I think that we need to specify what is the architecture string here.

Is it target triple? Is it value passed to -fsycl-targets? Is it value from architecture enum from our device architecture extension?

It is not clear how RT can use this field without such specifiation.

[steffenlarsen]

That is actually a good question. I am not sure what the architecture string would be for cases like SASS binaries. For example, lets say we've compiled to PTX through our compiler, then load that to a kernel-bundle, compile that kernel bundle to native device code and then serialize that to SYCLBIN. The -fsycl-targets would not be enough to express the architecture here, I believe.

[gmlueck]

In your example, the application has compiled the PTX to native code. Wouldn't you know the native architecture when this happens? It seems like the set of possible native CUDA architectures is a fixed set which would each map to one of the -fsycl-targets values.

I wonder if there is a reason to use a string for the architecture names. Why couldn't this be an enumeration? We use an enumeration for the device architectures in sycl_ext_oneapi_device_architecture.

In any case, I agree with @AlexeySachkov. I think the set of possible architectures should be specified in the file format.

[steffenlarsen]

In your example, the application has compiled the PTX to native code. Wouldn't you know the native architecture when this happens? It seems like the set of possible native CUDA architectures is a fixed set which would each map to one of the -fsycl-targets values.

I will have to do some research here. I know PTX can be associated with SM architectures, but I don't know if the same applies to the native device code produced from PTX. It may be device-specific and as such more strict than the SM version.

I wonder if there is a reason to use a string for the architecture names. Why couldn't this be an enumeration? We use an enumeration for the device architectures in sycl_ext_oneapi_device_architecture.

Since the compiler will need to know about these architectures too, I am reluctant to try and match enum values between the runtime and library for this purpose.

[steffenlarsen]

I've specified that the "arch" value is the string that corresponds to the -fsycl-targets value used for the binary. The runtime should generally be able to convert that to the enums in sycl_ext_oneapi_device_architecture.

[AlexeySachkov]

I've specified that the "arch" value is the string that corresponds to the -fsycl-targets value used for the binary. The runtime should generally be able to convert that to the enums in sycl_ext_oneapi_device_architecture.

The approach works for me, but I feel like the clarification is actually missing from the doc.

[steffenlarsen]

Good point! I've added the reference to the SYCL extension with a note about the runtime attempting to make the appropriate mapping.

[AlexeySachkov]

So, .syclbin files cannot be used if the output is not .syclbin, right?

I'm not sure if I have a use case for that, just wanted to double-check the intent.

A potential use-case, though, is ability to embed .syclbin into an application as if that device code was originally compiled as part of the application. I.e. you had your dynamically loadable .syclbin, but at some point decided to embed it and stop shipping it separately. But that will have some implications on the API, I assume: we need to design then how to use such embedded SYCLBIN.

[gmlueck]

I agree that it seems like we would probably want to annotate the IR module with the CUDA virtual architecture in the case when the IR is PTX. I was thinking that we would use the IR-level metadata for stuff like this.

[gmlueck]

In your example, the application has compiled the PTX to native code. Wouldn't you know the native architecture when this happens? It seems like the set of possible native CUDA architectures is a fixed set which would each map to one of the -fsycl-targets values.

I wonder if there is a reason to use a string for the architecture names. Why couldn't this be an enumeration? We use an enumeration for the device architectures in sycl_ext_oneapi_device_architecture.

In any case, I agree with @AlexeySachkov. I think the set of possible architectures should be specified in the file format.

[steffenlarsen]

@AlexeySachkov @asudarsa @mdtoguchi - I believe I've addressed the open comments. A re-review would be much appreciated. 😄

[steffenlarsen]

@intel/dpcpp-doc-reviewers @AlexeySachkov @asudarsa - Friendly ping.

[AlexeySachkov]

A few final comments here and there, but nothing major, I believe

[AlexeySachkov]

Not sure if this is a problem, but what if we add a new property there which cannot be applied to an abstract module?

[steffenlarsen]

I don't think adding new cases should be a problem. It is mostly to say that these property sets have no meaning for the abstract module metadata block. I don't think it's worth checking whether they are there or not, over just skipping them.

[AlexeySachkov]

I've specified that the "arch" value is the string that corresponds to the -fsycl-targets value used for the binary. The runtime should generally be able to convert that to the enums in sycl_ext_oneapi_device_architecture.

The approach works for me, but I feel like the clarification is actually missing from the doc.

[AlexeySachkov]

Could you please clarify what do you mean by "override -fsycl-device-only"? Isn't skipping host compilation implies -fsycl-device-only?

[steffenlarsen]

I suppose so. To some extend I think the terminology overlaps a little. They are not really mutually exclusive, it is just "-fsycl-device-only with more". I'll change it to just "implies". @mdtoguchi - Is there a precedence for the terminology used here?

[mdtoguchi]

I think the intention of the wording here is that use of -fsyclbin will only produce the SYCLBIN file. Expectation of using -fsycl-device-only is produce just the device binary. If they are used together, -fsycl-device-only is overridden from a driver function perspective.

[mdtoguchi]

Now that I see this written, what is the expectation of behavior when -fsyclbin -fsycl-device-only is used on the command line? The natural behavior here is for -fsycl-device-only to be seen and taken advantage of, which will create the device only file and stop. -fsyclbin does additional work after the device compilation.

[steffenlarsen]

That's a good point. Maybe it would be better to issue a diagnostic if they are used together? Seems like they may not be totally related, despite there being some conceptual overlap.

[mdtoguchi]

I think the standard 'unused argument' diagnostic should be sufficient when used together:

> clang++ --offload-new-driver -fsycl-device-only -fsyclbin ~/a.cpp
clang++: warning: argument unused during compilation: '-fsyclbin' [-Wunused-command-line-argument]

[steffenlarsen]

Sounds good! I've reworded this part to explicitly say that the -fsyclbin will be unused.

[asudarsa]

What is meant by post-processing here? Thanks

[steffenlarsen]

Module-splitting and metadata analysis/extraction mainly. I've expanded it a bit.

[asudarsa]

We are in the process of moving most of the SYCL specific functionality from clang-linker-wrapper into a new tool called clang-sycl-linker. So, this documentation will need to be updated based on that. For the purposes of this PR, we can use clang-linker-wrapper.
Just heads up.

[steffenlarsen]

Thanks for the heads up! Would it make sense to change it now? From a documentation POV, is it as simple as a search-and-replace or is there an important semantic difference between the tools?

[maksimsab]

Testing and debugging would require some capabilities of searching/extracting information from files of this format.
It would require us to provide such tools/utilities.
I think that making this format based on ELF would allows us to reuse some available utilities like standard gnu packages (readelf, objdump and i.e) and llvm utilities (llvm-objdump and rich LLVM library). LLVM library's support of ELF could be reused in the development as well.

Custom binary format requires custom support in many ways that significantly burdens the development and maintaining. I think we should strive to the generic Offloading Format from LLVM as much as we can.

What do you think?

[steffenlarsen]

I do agree with the general sentiment and I am open to the idea of reusing ELF as the format. However, I don't see how this format fits with ELF, as we would just be fitting bogus into a lot of the pre-defined ELF headers and sections. SYCLBIN is not an executable format per-se and to do appropriate linking the linker will have to consider the binary metadata too, which we would have to retrofit into some text section of the ELF file.

For tooling, I could see it, but are there any other tools than llvm-objdump (and readelf and objdump) that we would get "for free" if we used ELF? Even if we do, would users be able to get much out of the metadata without us adding additional functionality to these tool or entirely new tooling? When you mention "rich LLVM library" could you be more specific?

I've previously tried and failed to fit the SYCLBIN format into the existing ELF format in a way that isn't just what the current design is but separated into best-effort chunks in the ELF format, so please, if you have a suggestion of how to structure the format based off ELF, please do explain your thoughts.

[maksimsab]

There is our fault of not properly communicating the status of upstreaming of PropertySets.

There has been an attempt of doing that: llvm/llvm-project#110771
And there is comment/request to use text format (JSON or YAML):
llvm/llvm-project#110771 (comment)

In overall, we doubt that we able to get PropertySets in llvm-project as it is. At the moment, Justin is experimenting with text formats to add this into intel/llvm. Therefore, it is not provident to make the old binary format of PropertySet the part of a new ABI.

In these document we could describe properties in an abstract way as a collection of string keys and int/string values without specifying the exact binary format.

In the syclbin format we could add some field specifying the actual format like:

struct Properties {
  std::byte format[8];  // "json", "yaml", "prop" (old binary properties) and i.e.
  uint32_t size;
  std::byte encoded_data[size];
}

I don't insist on this concrete structure.

[steffenlarsen]

I don't think switching the format would be a problem. SYCLBIN just embeds the same properties as the compiler generates. This document is not intended to document a constituent of SYCLBIN but rather the current design of the property sets, which were not well documented. If we change to some other properties format, it should be straight forward to just switch over to embedding that instead, as they are simply encoded into the byte tables.

[maksimsab]

Yes. I mean that a documentation format similar to AMD's (link) table format wouldn't require an additional work in the future.

[steffenlarsen]

The switch to ELF-based format was discussed offline and we decided to go with the current design for now to enable the development of the feature and then consider the upstreamability in parallel.

[maksimsab]

LGTM.

PropertySets could be transformed to table later.