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

Feature/refactor program #845

Merged
merged 14 commits into from
Jun 2, 2022
Merged

Feature/refactor program #845

merged 14 commits into from
Jun 2, 2022

Conversation

oleksandr-pavlyk
Copy link
Collaborator

This change does away with use of deprecated sycl::program class replacing it with use of sycl::kernel_bundle<sycl::bundle_state::executable> instead.

DPCTLSyclProgramRef has been removed, but DPCTLSyclKernelBundleRef introduced, being an opaque reference to the executable kernel bundle as stated above.

Functions DPCTLProgram_* are renamed to DPCTLKernelBundle_*.

Free functions DPCTLKernelBundle_Create* now take context and device references (previously only took DPCTLSyclContextRef argument).

Implementation of dpctl_sycl_program_interface no longer directly uses OpenCL symbols, uses dynamic_Lib helper class to load them dynamically. Thus SyclInterface library no longer links with OpenCL library.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • If this PR is a work in progress, are you filing the PR as a draft?

@github-actions
Copy link

@coveralls
Copy link
Collaborator

coveralls commented May 23, 2022

Coverage Status

Coverage decreased (-1.3%) to 81.559% when pulling afd12ef on feature/refactor-program into 3340f33 on master.

@oleksandr-pavlyk oleksandr-pavlyk force-pushed the feature/refactor-program branch 2 times, most recently from 709c97b to 5746028 Compare May 24, 2022 13:18
Removed uses of `sycl::program` throughout the code.

In SyclInterface replace DPCTLSyclProgramRef with DPCTLSyclKernelBundleRef
which is reference to `sycl::kernel_bundle<sycl::bundle_state::executable>`.

Functions `DPCTLProgram_*` were replaced with `DCPTLKernelBundle_*`.
Functions to create program now take both context and device.

Tests were modified.

dpctl.SyclProgram stays with this name, but it now encapsulates
DPCTLSyclKernelBundleRef instead of removed DPCTLSyclProgramRef.

OpenCL functions are no longer directly used (instead of used via loader,
like in the case of level-zero backend), hence removed linkage to OpenCL
library.
Make sure to exercise addressof_ref() methods of SyclProgram and
SyclKernel.

Add an example of invalid source code to trip throwing of an exception.
@oleksandr-pavlyk
Copy link
Collaborator Author

@diptorupd As far as I can see this change show not require companion changes in numba-dpex.

With this change, removed use of -Wno-deprecated-declarations, since
code no longer uses any.
@oleksandr-pavlyk oleksandr-pavlyk force-pushed the feature/refactor-program branch from e9fd855 to b4d2d8b Compare May 24, 2022 18:17
Added a test for non-supported backend (host). This only runs if
SYCL_ENABLE_HOST_DEVICE environment variable is set.
Also parametrized test for compiling from source to run on OCL gpu
and OCL cpu if found.
@oleksandr-pavlyk
Copy link
Collaborator Author

Functions DPCTLKernelBundle_Create* dispatch to implementations for respective backends. Backend entity is created (either cl_program or ze_module_handle_t), which are used in sycl::make_kernel_bundle to create the interoperability kernel bundle.

Since interoperability kernel_bundle does not expose kernel ids for kernels in it, GetKernel and HasKernel functions are implementing by getting the native component out. This native component is a vector of cl_program or ze_module_handle_t.

For each of this the backend-appropriate function is used to look-up the kernel by its name.

SYCL does not guarantee uniqueness of the kernel with the given name in a kernel_bundle. The functionality retrieves the first match found.

Kernel bundles constructed from OCL source or from a single SPV are guaranteed to not have duplicates.

diptorupd
diptorupd previously approved these changes Jun 2, 2022
Copy link
Contributor

@diptorupd diptorupd left a comment

Choose a reason for hiding this comment

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

@oleksandr-pavlyk Overall it looks good. There may be a way to make the dpctl_sycl_kernel_bundle_interface.cpp more modular. E.g. having an interface and then each implementation of the interface OCL, ZE implementing the interface. But, we may soon find ourselves reinventing the DPC++ PI. So thanks, but no thanks!

Other things, I mostly glanced through so might have missed few things. If you are satisfied please feel free to merge.

@oleksandr-pavlyk oleksandr-pavlyk merged commit 45db6ea into master Jun 2, 2022
@oleksandr-pavlyk oleksandr-pavlyk deleted the feature/refactor-program branch June 2, 2022 15:05
@github-actions
Copy link

github-actions bot commented Jun 2, 2022

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

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.

3 participants