Skip to content
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

[Fusion] Add fusion JIT integration design proposal #7204

Merged
merged 3 commits into from
Mar 14, 2023

Conversation

Naghasan
Copy link
Contributor

Design document covering the approach to integrate the kernel fusion extension into the runtime and
the kernel fusion JIT process.

Covers design to implement extension proposed in #7098

Signed-off-by: Victor Lomuller victor@codeplay.com
Co-authored-by: Lukas Sommer lukas.sommer@codeplay.com
Co-authored-by: Victor Perez victor.perez@codeplay.com

@Naghasan Naghasan requested a review from pvchupin October 27, 2022 14:41
@Naghasan Naghasan requested a review from a team as a code owner October 27, 2022 14:41
Design document covering the approach to integrate
the kernel fusion extension into the runtime and
the kernel fusion JIT process.

Signed-off-by: Victor Lomuller <victor@codeplay.com>
Co-authored-by: Lukas Sommer <lukas.sommer@codeplay.com>
Co-authored-by: Victor Perez <victor.perez@codeplay.com>
@Naghasan Naghasan force-pushed the kernel-fusion-design branch from 8e0fefc to 367a96e Compare October 27, 2022 15:17
@Naghasan Naghasan requested a review from a team as a code owner October 27, 2022 15:17
sycl/doc/design/KernelFusionJIT.md Outdated Show resolved Hide resolved
sycl/doc/design/KernelFusionJIT.md Outdated Show resolved Hide resolved
sycl/doc/design/KernelFusionJIT.md Outdated Show resolved Hide resolved
sycl/doc/design/KernelFusionJIT.md Outdated Show resolved Hide resolved
sycl/doc/design/KernelFusionJIT.md Outdated Show resolved Hide resolved
sycl/doc/design/KernelFusionJIT.md Outdated Show resolved Hide resolved
sycl/doc/design/KernelFusionJIT.md Outdated Show resolved Hide resolved
sycl/doc/design/KernelFusionJIT.md Outdated Show resolved Hide resolved
sycl/doc/design/KernelFusionJIT.md Outdated Show resolved Hide resolved
sycl/doc/design/KernelFusionJIT.md Outdated Show resolved Hide resolved
@Naghasan
Copy link
Contributor Author

Naghasan commented Nov 1, 2022

@bader Thanks for the review and comments. I pushed a new revision of the document addressing your comments.

@Naghasan
Copy link
Contributor Author

Ping

@pvchupin pvchupin requested review from bader and gmlueck November 21, 2022 19:28
steffenlarsen pushed a commit that referenced this pull request Nov 24, 2022
This is the first patch in a series of patches to add an implementation
of the [kernel fusion
extension](#7098). We have split the
implementation into multiple patches to make them more easy to review.

This first patch introduces the user-facing API discussed in the
[extension proposal](#7098). It does
not yet add any fusion functionality, just the mere API and SYCL
properties. Calls to the API will throw an error until the remaining
functionality lands with the following patches.

The design document for the overall implementation of kernel fusion can
be found [here](#7204).

Signed-off-by: Lukas Sommer <lukas.sommer@codeplay.com>
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

@bader Thanks for the review and comments. I pushed a new revision of the document addressing your comments.

LGTM, thanks!
I'd like someone from @intel/dpcpp-specification-reviewers to approve before merging. @gmlueck, could you take a look, please?

againull pushed a commit that referenced this pull request Dec 16, 2022
This is the third patch in a series of patches to add an implementation
of the [kernel fusion
extension](#7098). We have split the
implementation into multiple patches to make them more easy to review.
This patch integrates the kernel fusion extension into the SYCL runtime
scheduler.

Next to collecting the kernels submitted while in fusion mode in the
fusion list associated with the queue, the integration into the
scheduler is also responsible for detecting the synchronization
scenarios. Various scenarios, such as buffer destruction or event wait,
require fusion to be aborted early. The full list of scenarios is
available in the [extension
proposal](https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/experimental/sycl_ext_codeplay_kernel_fusion.asciidoc#synchronization-in-the-sycl-application).

A high-level description of the integration into the scheduler can be
found in the [design document](#7204).

This PR can be reviewed and merged independently of
#7465.

Signed-off-by: Lukas Sommer <lukas.sommer@codeplay.com>

Signed-off-by: Lukas Sommer <lukas.sommer@codeplay.com>
steffenlarsen pushed a commit that referenced this pull request Jan 13, 2023
This is the fifth patch in a series of patches to add an implementation
of the [kernel fusion
extension](#7098). We have split the
implementation into multiple patches to make them more easy to review.

This patch connects the JIT compiler for kernel fusion (`sycl-fusion`)
with the SYCL runtime.

- Enable the feature by default and add an option to `configure.py` to
disable it.
- Link the runtime against the JIT compiler library as a shared library.
- Add logic to retrieve binaries (SPIR-V) and other information (e.g.,
accessors) from the SYCL RT and invoke the JIT compiler.
- Representation to store binaries (SPIR-V) returned by JIT compiler in
memory for use as PI device binaries.

The integration of the JIT compiler into the SYCL RT is described in
[this design document](#7204).

Signed-off-by: Lukas Sommer <lukas.sommer@codeplay.com>
@bader bader removed the request for review from pvchupin January 25, 2023 02:14
@bader
Copy link
Contributor

bader commented Jan 25, 2023

Resolved merge conflicts.
@intel/dpcpp-specification-reviewers, @gmlueck, could you take a look, please?

@Naghasan
Copy link
Contributor Author

Naghasan commented Mar 6, 2023

Ping :)

@gmlueck
Copy link
Contributor

gmlueck commented Mar 6, 2023

https://github.com/orgs/intel/teams/dpcpp-specification-reviewers, @gmlueck, could you take a look, please?

I think this falls into the same category as the XPTI design document PR. It seems like our decision is that the development team should be primarily responsible for reviewing design docs, and they should the @intel/dpcpp-specification-reviewers for input if there is a specific question about conformance to the language spec. If that is our direction, then I think the development team should be primary reviewer for this PR.

@sommerlukas
Copy link
Contributor

As @gmlueck suggests that the development team should be the primary reviewer for this PR and the PR has been approved by @bader, can we have this merged?

@Naghasan
Copy link
Contributor Author

@gmlueck Lukas has approved the PR, can you do also approve so that @bader can merge ? thanks

Copy link
Contributor

@gmlueck gmlueck left a comment

Choose a reason for hiding this comment

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

I have not reviewed this because we agreed that @intel/dpcpp-specification-reviewers do not need to approve changes to design documents. I'm approving only to allow @bader to merge.

@bader bader merged commit 00f1ca7 into intel:sycl Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants