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

[SYCL][Graph] Implementation of explicit update with indices #12840

Merged
merged 32 commits into from
Mar 22, 2024

Conversation

Bensuo
Copy link
Contributor

@Bensuo Bensuo commented Feb 27, 2024

- Experimental implementation of explicit update with indices
- New scheduler command for updating a command buffer command
- PI equivalents for new UR APIs
- E2E and Unit Tests
@Bensuo Bensuo requested review from a team as code owners February 27, 2024 17:15
EwanC added a commit to Bensuo/unified-runtime that referenced this pull request Feb 27, 2024
Implement the API for updating the kernel commands in a command-buffer
defined by oneapi-src#1089 for
the OpenCL adapter.

However, the following changes to the UR kernel update API have been
made based on implementation experience:

1. Forbid updating the work-dim of the kernel, see KhronosGroup/OpenCL-Docs#1057
2. Remove struct fields to update exec info, after [DPC++ implementation
   prototype](intel/llvm#12840) shows this isn't
   needed.

This adapter implementation depends on support for the
[cl_khr_command_buffer_mutable_dispatch](https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_Ext.html#cl_khr_command_buffer_mutable_dispatch)
extension.

Tested on Intel GPU/CPUs OpenCL implementations with the
[command-buffer emulation
layer](https://github.com/bashbaug/SimpleOpenCLSamples/tree/main/layers/10_cmdbufemu).

```bash
$ OPENCL_LAYERS=<path/to/SimpleOpenCLSamples/build/layers/10_cmdbufemu/libCmdBufEmu.so> ./bin/test-exp_command_buffer --platform="Intel(R) OpenCL Graphics"
```

DPC++ PR intel/llvm#12724
Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

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

Hi, could you also update sycl/test/abi/pi_nativecpu_symbol_check.dump, same changes you have done for the other symbol check files, thanks

sycl/source/detail/graph_impl.hpp Outdated Show resolved Hide resolved
sycl/source/detail/scheduler/scheduler.hpp Show resolved Hide resolved
sycl/source/detail/scheduler/scheduler.hpp Show resolved Hide resolved
@Bensuo Bensuo changed the title [SYCL][Graph] Prototype of explicit update with indices [SYCL][Graph] Implementation of explicit update with indices Mar 7, 2024
@Bensuo
Copy link
Contributor Author

Bensuo commented Mar 20, 2024

@intel/unified-runtime-reviewers @intel/dpcpp-tools-reviewers Could I get some reviews on this PR? Thanks!

Copy link
Contributor

@aarongreig aarongreig left a comment

Choose a reason for hiding this comment

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

plugin changes LGTM

Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

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

Native CPU LGTM, thank you

@EwanC
Copy link
Contributor

EwanC commented Mar 22, 2024

@AlexeySachkov Could you review on behalf of @intel/dpcpp-tools-reviewers ? Thanks

Copy link
Contributor

@AlexeySachkov AlexeySachkov left a comment

Choose a reason for hiding this comment

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

DeviceConfigFile.td LGTM

Copy link
Contributor

github-actions bot commented Mar 22, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@EwanC
Copy link
Contributor

EwanC commented Mar 22, 2024

@intel/llvm-gatekeepers This is good to merge, thanks

@sarnex sarnex merged commit 2bc8b5b into intel:sycl Mar 22, 2024
14 checks passed
EwanC added a commit to Bensuo/unified-runtime that referenced this pull request Mar 25, 2024
Implement the API for updating the kernel commands in a command-buffer
defined by oneapi-src#1089 for
the OpenCL adapter.

However, the following changes to the UR kernel update API have been
made based on implementation experience:

1. Forbid updating the work-dim of the kernel, see KhronosGroup/OpenCL-Docs#1057
2. Remove struct fields to update exec info, after [DPC++ implementation
   prototype](intel/llvm#12840) shows this isn't
   needed.
3. Forbid changing the local work size from user to impl defined and
   vice-versa. See discussion in [L0 implementation
PR](oneapi-src#1353 (comment)).

This adapter implementation depends on support for the
[cl_khr_command_buffer_mutable_dispatch](https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_Ext.html#cl_khr_command_buffer_mutable_dispatch)
extension.

Tested on Intel GPU/CPUs OpenCL implementations with the
[command-buffer emulation
layer](https://github.com/bashbaug/SimpleOpenCLSamples/tree/main/layers/10_cmdbufemu).

```bash
$ OPENCL_LAYERS=<path/to/SimpleOpenCLSamples/build/layers/10_cmdbufemu/libCmdBufEmu.so> ./bin/test-exp_command_buffer --platform="Intel(R) OpenCL Graphics"
```

DPC++ PR intel/llvm#12724
sommerlukas pushed a commit that referenced this pull request Mar 27, 2024
- Adds APIs to the specification for updating graph node arguments using
explicit indices (from `set_arg()` etc.)
- Also includes functionality for updating ND-range of kernel nodes
- Note: Current design is only for kernel execution nodes

Implementation in #12840

---------

Co-authored-by: Pablo Reble <pablo.reble@intel.com>
Co-authored-by: Ewan Crawford <ewan@codeplay.com>
EwanC added a commit to Bensuo/unified-runtime that referenced this pull request Apr 4, 2024
Implement the API for updating the kernel commands in a command-buffer
defined by oneapi-src#1089 for
the OpenCL adapter.

However, the following changes to the UR kernel update API have been
made based on implementation experience:

1. Forbid updating the work-dim of the kernel, see KhronosGroup/OpenCL-Docs#1057
2. Remove struct fields to update exec info, after [DPC++ implementation
   prototype](intel/llvm#12840) shows this isn't
   needed.
3. Forbid changing the local work size from user to impl defined and
   vice-versa. See discussion in [L0 implementation
PR](oneapi-src#1353 (comment)).

This adapter implementation depends on support for the
[cl_khr_command_buffer_mutable_dispatch](https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_Ext.html#cl_khr_command_buffer_mutable_dispatch)
extension.

Tested on Intel GPU/CPUs OpenCL implementations with the
[command-buffer emulation
layer](https://github.com/bashbaug/SimpleOpenCLSamples/tree/main/layers/10_cmdbufemu).

```bash
$ OPENCL_LAYERS=<path/to/SimpleOpenCLSamples/build/layers/10_cmdbufemu/libCmdBufEmu.so> ./bin/test-exp_command_buffer --platform="Intel(R) OpenCL Graphics"
```

DPC++ PR intel/llvm#12724
EwanC added a commit to Bensuo/unified-runtime that referenced this pull request Apr 5, 2024
Implement the API for updating the kernel commands in a command-buffer
defined by oneapi-src#1089 for
the OpenCL adapter.

However, the following changes to the UR kernel update API have been
made based on implementation experience:

1. Forbid updating the work-dim of the kernel, see KhronosGroup/OpenCL-Docs#1057
2. Remove struct fields to update exec info, after [DPC++ implementation
   prototype](intel/llvm#12840) shows this isn't
   needed.
3. Forbid changing the local work size from user to impl defined and
   vice-versa. See discussion in [L0 implementation
PR](oneapi-src#1353 (comment)).

This adapter implementation depends on support for the
[cl_khr_command_buffer_mutable_dispatch](https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_Ext.html#cl_khr_command_buffer_mutable_dispatch)
extension.

Tested on Intel GPU/CPUs OpenCL implementations with the
[command-buffer emulation
layer](https://github.com/bashbaug/SimpleOpenCLSamples/tree/main/layers/10_cmdbufemu).

```bash
$ OPENCL_LAYERS=<path/to/SimpleOpenCLSamples/build/layers/10_cmdbufemu/libCmdBufEmu.so> ./bin/test-exp_command_buffer --platform="Intel(R) OpenCL Graphics"
```

DPC++ PR intel/llvm#12724
kbenzie pushed a commit to kbenzie/llvm that referenced this pull request Apr 18, 2024
Add cubemap support:
 - Allocation and freeing of cubemapped images
 - Unsampled fetching and writing, and sampled reading
 - Device queries for cubemap support
 - Testing for both unsampled and sampled cubemap examples
 - Update the spec with cubemap support

Remove `const` and `&` qualifiers from spec and implementation for
handle parameters in `write_xxx` functions.

Corresponding UR PR:
oneapi-src/unified-runtime#1433

---------

Co-authored-by: Przemek Malon <przemek.malon@codeplay.com>

Resolved Conflicts in:
- Due to not cherry-picking intel#12840
  - sycl/include/sycl/detail/pi.h
- Due to not cherry-picking intel#13181
  - sycl/include/sycl/device_aspect_macros.hpp
  - sycl/include/sycl/info/aspects.def
EwanC added a commit to Bensuo/unified-runtime that referenced this pull request Apr 22, 2024
Implement the API for updating the kernel commands in a command-buffer
defined by oneapi-src#1089 for
the OpenCL adapter.

However, the following changes to the UR kernel update API have been
made based on implementation experience:

1. Forbid updating the work-dim of the kernel, see KhronosGroup/OpenCL-Docs#1057
2. Remove struct fields to update exec info, after [DPC++ implementation
   prototype](intel/llvm#12840) shows this isn't
   needed.
3. Forbid changing the local work size from user to impl defined and
   vice-versa. See discussion in [L0 implementation
PR](oneapi-src#1353 (comment)).

This adapter implementation depends on support for the
[cl_khr_command_buffer_mutable_dispatch](https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_Ext.html#cl_khr_command_buffer_mutable_dispatch)
extension.

Tested on Intel GPU/CPUs OpenCL implementations with the
[command-buffer emulation
layer](https://github.com/bashbaug/SimpleOpenCLSamples/tree/main/layers/10_cmdbufemu).

```bash
$ OPENCL_LAYERS=<path/to/SimpleOpenCLSamples/build/layers/10_cmdbufemu/libCmdBufEmu.so> ./bin/test-exp_command_buffer --platform="Intel(R) OpenCL Graphics"
```

DPC++ PR intel/llvm#12724
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.