Enable level zero v2 in DPC++ #16656

omarahmed1111 · 2025-01-16T10:30:02Z

This makes a simple way to use the experimental level_zero_v2 in dpc++. Fixes #16613 .

pbalcer · 2025-01-16T11:36:53Z

My opinion is that we should build and ship both adapters by default, and make it togglable which one is used at runtime through an env variable (like I suggested here).
Having both is often useful for performance comparisons, or simply to have a fallback if one of the adapters is not behaving correctly.

omarahmed1111 · 2025-01-16T14:51:34Z

My opinion is that we should build and ship both adapters by default, and make it togglable which one is used at runtime through an env variable (like I suggested here). Having both is often useful for performance comparisons, or simply to have a fallback if one of the adapters is not behaving correctly.

Good point! changed that.

jbrodman · 2025-01-16T17:16:46Z

By default, I think we should only build 1 adapter. We don't want to confuse users by enumerating the same device even more times than we already do (OpenCL vs Level Zero). I think it would be nice to support building both and toggling for the reasons stated above, but I'd also like the option to ONLY build v1 or v2.

pbalcer · 2025-01-17T09:17:36Z

By default, I think we should only build 1 adapter. We don't want to confuse users by enumerating the same device even more times than we already do (OpenCL vs Level Zero). I think it would be nice to support building both and toggling for the reasons stated above, but I'd also like the option to ONLY build v1 or v2.

The current proposal, and, from what I can tell, the implementation, is that both adapters will be physically present in the libs directory, however, only one of them will be active at a time. There will be a new environment variable (UR_ADAPTER_LEVEL_ZERO_V2=1, although, long-term, I'd have preferred for this to be an option for ONEAPI_DEVICE_SELECTOR), that users will be able to set to toggle which adapter is used. This ensures that L0 devices are not enumerated twice, but makes it very simple for users to experiment with the new adapter.

Having to build SYCL from scratch with a special option might be challenging for developers of higher-level frameworks or applications that usually use a prebuilt compiler.

sarnex · 2025-01-17T16:08:28Z

buildbot/configure.py

@@ -68,6 +68,7 @@ def do_configure(args):

    if sys.platform != "darwin":
        sycl_enabled_backends.append("level_zero")
+        sycl_enabled_backends.append("level_zero_v2")


Couple of comments

Can you explain what L0 V2 is and how it releases to the current L0/L0 adapter

I agree with James, we shouldn't build two plugins by default. If we have some configure switch to build two, that's fine.

README. In short, it's a new L0 adapter with re-implemented performance-critical APIs. See here for performance validation results.

Why though? As I noted, with the current implementation, users won't be affected by the inclusion of the two adapters. And not including it by default would mean that it's going to be much more difficult for application and framework developers to experiment with the new adapter, which is the whole point of including it in SYCL right now in the first place. In practice, it will likely mean we might need to provide the users that want to use it an alternative prebuilt compiler package.

@igchor ping

Out of curiosity, why "v2" instead of incremental refactoring?

Got it, thanks

At this point it seems like the v2 adapter is only intended to be used by DPCPP/UR developers, and IMO features like that should not be built by default. If the feature is ready enough for downstream clients, should we switch over by default?

Out of curiosity, why "v2" instead of incremental refactoring?

The existing L0 adapter code base has accumulated a lot of complexity over the years as L0 and the driver evolved. It has become very difficult to change. For example, there are 4 major queue modes that the adapter supports - "batched in order", "batched out of order", "immediate in order", "immediate out of order". In the existing adapter, they are all implemented together, with various conditions sprinkled all throughout to make it work. In practice, batched mode is all but legacy for 99% of scenarios, and "in order" and "out of order" L0 paths are so different (different types of L0 events, different way of synchronization), that there's not much to gain from sharing all the code between them.
Refactoring this incrementally, without breaking existing code, would be challenging. So the team has decided that a partial re-implementation (we still reuse all that's practical) of the performance critical paths is the best path forward. This also means that we have the opportunity to easily leverage new driver features and do a clean break away from the accumulated legacy tech debt.

At this point it seems like the v2 adapter is only intended to be used by DPCPP/UR developers, and IMO features like that should not be built by default. If the feature is ready enough for downstream clients, should we switch over by default?

In terms of quality, the v2 adapter has higher UR CTS passrate than the current one, and passes the vast majority of SYCL e2e tests (big exception being tests that hard-code exact sequence of operations expected from the adapter). However, this is still work-in-progress.

pbalcer · 2025-01-29T11:47:19Z

sycl/cmake/modules/FetchUnifiedRuntime.cmake

@@ -36,6 +36,10 @@ set(UR_ENABLE_TRACING ON)
 if("level_zero" IN_LIST SYCL_ENABLE_BACKENDS)
  set(UR_BUILD_ADAPTER_L0 ON)
 endif()
+if("level_zero_v2" IN_LIST SYCL_ENABLE_BACKENDS)
+  set(UR_BUILD_ADAPTER_L0_V2 ON)
+  set(ENV{UR_ADAPTER_LEVEL_ZERO_V2} ON)


This will only set the env variable for the build process. We either need to change the ur loader again is that the v2 is loaded unconditionally (but i'm not sure about that, the L0 adapter might be in other searchable OS paths, and we'd end up with two adapters anyway) or set this env variable somewhere in SYCL rt init if this compile option is set.

I tried to think in an easy way to do the second option and make the compile option consistent in defining the env variable, but I think we will always need the user to define the UR_ADAPTER_LEVEL_ZERO_V2, I guess we could just depend on the user to define it, or we could define another dpc++ env variable that the user could define if that would make it more clear.

The first option seems the simpler one, but with the probability of reporting still 2 l0 adapters.

I am not sure which one is better, but I think we could just for simplicity go with the first suggestion of just registering the two l0/l0_v2 adapters, and let dpc++ control that with the compile option.

Makes sense, I agree. I already approved the UR change. Thanks.

igchor · 2025-01-29T21:59:28Z

@omarahmed1111 will this approach enable us to mark specific e2e tests with XFAIL for v2 only, or would that require some extra changes?

omarahmed1111 · 2025-01-30T12:13:47Z

@omarahmed1111 will this approach enable us to mark specific e2e tests with XFAIL for v2 only, or would that require some extra changes?

I guess that will need extra changes to lit tests config to be able to detect level_zero_v2 as a separate backend that could be marked as xfail, not sure if we need also to make sycl-rt detect level_zero_v2 as a valid backend for that to work.

pbalcer · 2025-01-30T12:15:46Z

@omarahmed1111 will this approach enable us to mark specific e2e tests with XFAIL for v2 only, or would that require some extra changes?

I guess that will need extra changes to lit tests config to be able to detect level_zero_v2 as a separate backend that could be marked as xfail, not sure if we need also to make sycl-rt detect level_zero_v2 as a valid backend for that to work.

Let's leave that for a separate PR. This one isn't adding any new CI job for v2 anyway.

omarahmed1111 force-pushed the enable-level-zero-v2 branch from 32fded7 to 9bc55cd Compare January 16, 2025 10:31

omarahmed1111 temporarily deployed to WindowsCILock January 16, 2025 10:31 — with GitHub Actions Inactive

omarahmed1111 temporarily deployed to WindowsCILock January 16, 2025 11:02 — with GitHub Actions Inactive

omarahmed1111 force-pushed the enable-level-zero-v2 branch from 9bc55cd to dd18cba Compare January 16, 2025 14:49

omarahmed1111 added 2 commits January 16, 2025 14:50

Enable level_zero_v2 on sycl

123099e

Update UR tag

cbbdcec

omarahmed1111 force-pushed the enable-level-zero-v2 branch from dd18cba to cbbdcec Compare January 16, 2025 14:51

omarahmed1111 temporarily deployed to WindowsCILock January 16, 2025 14:52 — with GitHub Actions Inactive

omarahmed1111 temporarily deployed to WindowsCILock January 16, 2025 15:33 — with GitHub Actions Inactive

kbenzie mentioned this pull request Jan 17, 2025

Enable the registry of level_zero_v2 oneapi-src/unified-runtime#2571

Open

omarahmed1111 marked this pull request as ready for review January 17, 2025 15:29

omarahmed1111 requested review from a team as code owners January 17, 2025 15:29

omarahmed1111 requested a review from againull January 17, 2025 15:29

sarnex reviewed Jan 17, 2025

View reviewed changes

omarahmed1111 added 2 commits January 29, 2025 11:06

Change to only build one adapter

436c0e6

Merge branch 'sycl' into enable-level-zero-v2

328b88f

omarahmed1111 temporarily deployed to WindowsCILock January 29, 2025 11:09 — with GitHub Actions Inactive

omarahmed1111 requested a review from pbalcer January 29, 2025 11:10

pbalcer reviewed Jan 29, 2025

View reviewed changes

omarahmed1111 had a problem deploying to WindowsCILock January 29, 2025 12:04 — with GitHub Actions Failure

Remove the l0 envvar assign

a80036d

omarahmed1111 temporarily deployed to WindowsCILock January 29, 2025 15:49 — with GitHub Actions Inactive

omarahmed1111 temporarily deployed to WindowsCILock January 29, 2025 16:53 — with GitHub Actions Inactive

omarahmed1111 added 2 commits January 30, 2025 12:18

formatting

d73e2f7

Merge branch 'sycl' into enable-level-zero-v2

85055dc

omarahmed1111 had a problem deploying to WindowsCILock January 30, 2025 12:19 — with GitHub Actions Failure

omarahmed1111 had a problem deploying to WindowsCILock January 30, 2025 12:26 — with GitHub Actions Failure

fix configure.py formatting

2cd1aa2

omarahmed1111 temporarily deployed to WindowsCILock January 30, 2025 12:32 — with GitHub Actions Inactive

omarahmed1111 had a problem deploying to WindowsCILock January 30, 2025 12:59 — with GitHub Actions Failure

omarahmed1111 had a problem deploying to WindowsCILock January 30, 2025 14:04 — with GitHub Actions Failure

omarahmed1111 temporarily deployed to WindowsCILock January 30, 2025 17:35 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable level zero v2 in DPC++ #16656

Enable level zero v2 in DPC++ #16656

omarahmed1111 commented Jan 16, 2025

pbalcer commented Jan 16, 2025 •

edited

Loading

omarahmed1111 commented Jan 16, 2025

jbrodman commented Jan 16, 2025

pbalcer commented Jan 17, 2025

sarnex Jan 17, 2025

pbalcer Jan 17, 2025 •

edited

Loading

aelovikov-intel Jan 17, 2025

sarnex Jan 17, 2025 •

edited

Loading

pbalcer Jan 17, 2025 •

edited

Loading

pbalcer Jan 29, 2025

omarahmed1111 Jan 29, 2025

pbalcer Jan 30, 2025

igchor commented Jan 29, 2025

omarahmed1111 commented Jan 30, 2025

pbalcer commented Jan 30, 2025

Enable level zero v2 in DPC++ #16656

Are you sure you want to change the base?

Enable level zero v2 in DPC++ #16656

Conversation

omarahmed1111 commented Jan 16, 2025

pbalcer commented Jan 16, 2025 • edited Loading

omarahmed1111 commented Jan 16, 2025

jbrodman commented Jan 16, 2025

pbalcer commented Jan 17, 2025

sarnex Jan 17, 2025

Choose a reason for hiding this comment

pbalcer Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

aelovikov-intel Jan 17, 2025

Choose a reason for hiding this comment

sarnex Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

pbalcer Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

pbalcer Jan 29, 2025

Choose a reason for hiding this comment

omarahmed1111 Jan 29, 2025

Choose a reason for hiding this comment

pbalcer Jan 30, 2025

Choose a reason for hiding this comment

igchor commented Jan 29, 2025

omarahmed1111 commented Jan 30, 2025

pbalcer commented Jan 30, 2025

pbalcer commented Jan 16, 2025 •

edited

Loading

pbalcer Jan 17, 2025 •

edited

Loading

sarnex Jan 17, 2025 •

edited

Loading

pbalcer Jan 17, 2025 •

edited

Loading