Skip to content

Conversation

rbegam
Copy link
Contributor

@rbegam rbegam commented Jul 20, 2020

Adding 'opencl' requirement disabled several tests some of which
have non-opencl constructs as well. This patch separates those tests
to enable testing of SYCL core functionalities.

Signed-off-by: rbegam [email protected]

Adding 'opencl' requirement disabled several tests some of which
have non-opencl constructs as well. This patch separates those tests
to enable testing of SYCL core functionalities.

Signed-off-by: rbegam <[email protected]>
@rbegam rbegam requested a review from a team as a code owner July 20, 2020 23:32
@rbegam rbegam requested a review from vladimirlaz July 20, 2020 23:32
Signed-off-by: rbegam <[email protected]>
@rbegam rbegam requested a review from smaslov-intel July 20, 2020 23:46
@@ -1,4 +1,4 @@
// REQUIRES: opencl
// REQUIRES: opencl || level0
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder how clCreateUserEvent call below will be processed if only Level0 RT is available

Copy link
Contributor Author

Choose a reason for hiding this comment

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

clCreateUserEvent is not called for host and the test sets SYCL_DEVICE_TYPE=HOST. Do you suggest separating this test too?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not quite understand the logic of the test:

  1. if we force HOST device lines 26-31 are dead code.
  2. It looks like the whole test check host events. What is the reason for having OpenCL or Level0 BE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is true. I am modifying this.

Signed-off-by: rbegam <[email protected]>
rbegam added 2 commits July 21, 2020 16:37
@rbegam
Copy link
Contributor Author

rbegam commented Jul 28, 2020

ping @smaslov-intel @vladimirlaz

@pvchupin pvchupin requested a review from vladimirlaz July 28, 2020 19:19
@vladimirlaz
Copy link
Contributor

ping @smaslov-intel @vladimirlaz

see my comment above

smaslov-intel
smaslov-intel previously approved these changes Jul 30, 2020
Copy link
Contributor

@smaslov-intel smaslov-intel left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: rbegam <[email protected]>
@vladimirlaz vladimirlaz merged commit 4a1eb53 into intel:sycl Jul 31, 2020
@@ -1,7 +1,10 @@
// REQUIRES: opencl
// REQUIRES: opencl || level0
Copy link
Contributor

Choose a reason for hiding this comment

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

215f591 changed this feature name.

level0 -> level_zero

Please, fix ASAP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

up for review #2235

Chenyang-L pushed a commit that referenced this pull request Feb 18, 2025
To support multi-device AOT scenario in SYCL we need an ability to create UR
program from multiple device binaries.

Changes in this PR:
* Modify the core function `urProgramCreateWithBinary` to support program
  creation from multiple device binaries.
* Add methods to ur_program to get/set per-device data like L0 module handle,
  build log etc. Otherwise any change in the structure of the class requires
  multiple changes in all UR functions which work with ur_program, in addition
  to this it allows to handle interop case (which implementation is an
  exception right now) in a single place. Also make State and some other info
  per-device because it is possible that UR program is associated with multiple
  devices and urProgramBuildExp is called multiple times for subsets, and we
  have to know the state per-device.
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.

4 participants