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

Unified Runtime v0.8.3 changes #12302

Closed
wants to merge 25 commits into from
Closed

Unified Runtime v0.8.3 changes #12302

wants to merge 25 commits into from

Conversation

aarongreig and others added 19 commits December 18, 2023 08:57
Pulls in the following fixes to the CUDA and HIP adapters:

* [HIP] Correctly set HIP Kernel Buffer Arguments 
* [HIP] Fix host/device synchronization 
* [HIP][CTS] Fix Device CTS failures 
* [HIP] Fix get mem size segfault 
* [HIP] Implement urMemImageGetInfo
* [HIP] Define all UR entry points
* [CUDA] Add support for binary type query
* [CUDA] Update hint functions to only return warnings instead of errors
Includes UR changes from [ [OpenCL] Fix memory leak and coverity issue
with struct-to-array casts #1048
](oneapi-src/unified-runtime#1048).
…ompile (#11464)

piProgramBuild receives a list of devices, while urProgramBuild does
not. This produces a series of issues when a UR program needs to be
created for a specific device.

So define a new API, called urProgramBuildExp to pass this list.

Requires related patch in Unified Runtime Adapters here:
oneapi-src/unified-runtime#934

---------

Signed-off-by: Spruit, Neil R <neil.r.spruit@intel.com>
Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
In oneapi-src/unified-runtime#993 the type of
the `UR_OPENCL_ICD_LOADER_LIBRARY` CMake variable will change to a cache
variable, as such the usage of this variable also needs to up updated.
Use a new environment variable, UR_L0_LEAKS_DEBUG, to check for leaks in
the UR L0 adapter, instead of relying on a specific value being set in
UR_L0_DEBUG.

---------

Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
intel-llvm CI run for adding Command Buffers to the OpenCL Adapter in
Unified Runtime - oneapi-src/unified-runtime#966

Also completes follow-on work identified in #11599 to add an OpenCL
section to the SYCL-Graphs docs and update the e2e Graph tests. Updating
the tests has since been completed in a separate PR -
#11877

Depends on #11820 merging first.

---------

Co-authored-by: Pablo Reble <pablo@reble.org>
Co-authored-by: Ewan Crawford <ewan@codeplay.com>
Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
Run CI for oneapi-src/unified-runtime#936

Depends on #11718 merging first.

---------

Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
…11541)

When event list is null, a barrier is still needed for all previous
commands, so fix it.

---------

Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
…cs. (#12005)

Also handle translating these properties in pi2ur.

oneapi-src/unified-runtime#1123

---------

Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
Corresponding UR changes
oneapi-src/unified-runtime#1179

---------

Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
Corresponding UR chanages
oneapi-src/unified-runtime#1105.

---------

Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
This to use latest features present in L0 spec.

---------

Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
The code object finalization for kernel fusion uses the AMD COMGR. The
location of the corresponding header changed between ROCm version 4 and
5.

This PR fixes the include for ROCm version 4.

---------

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

github-actions bot commented Jan 5, 2024

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:
darker --check --diff -r 6a0692cd91b3f1f4c1797a2644422e5f7c1c2c62..dcfe7f8048895c5fae66df10679b375200060d39 sycl/test-e2e/format.py sycl/test-e2e/lit.cfg.py
View the diff from darker here.
--- format.py	2023-12-18 08:57:41.000000 +0000
+++ format.py	2024-01-22 15:00:41.637048 +0000
@@ -162,15 +162,21 @@
             # current llvm-lit invocation isn't configured to include it. We
             # don't use ONEAPI_DEVICE_SELECTOR for `%{run-unfiltered-devices}`
             # so that device might still be accessible to some of the tests yet
             # we won't set the environment variable below for such scenario.
             extra_env = []
-            if 'ext_oneapi_level_zero:gpu' in sycl_devices and litConfig.params.get('ur_l0_debug'):
-                extra_env.append('UR_L0_DEBUG={}'.format(test.config.ur_l0_debug))
-
-            if 'ext_oneapi_level_zero:gpu' in sycl_devices and litConfig.params.get('ur_l0_leaks_debug'):
-                extra_env.append('UR_L0_LEAKS_DEBUG={}'.format(test.config.ur_l0_leaks_debug))
+            if "ext_oneapi_level_zero:gpu" in sycl_devices and litConfig.params.get(
+                "ur_l0_debug"
+            ):
+                extra_env.append("UR_L0_DEBUG={}".format(test.config.ur_l0_debug))
+
+            if "ext_oneapi_level_zero:gpu" in sycl_devices and litConfig.params.get(
+                "ur_l0_leaks_debug"
+            ):
+                extra_env.append(
+                    "UR_L0_LEAKS_DEBUG={}".format(test.config.ur_l0_leaks_debug)
+                )
 
             if 'ext_oneapi_cuda:gpu' in sycl_devices:
                 extra_env.append('SYCL_PI_CUDA_ENABLE_IMAGE_SUPPORT=1')
 
             return extra_env
--- lit.cfg.py	2023-12-18 08:57:41.000000 +0000
+++ lit.cfg.py	2024-01-22 15:00:41.849029 +0000
@@ -133,19 +133,19 @@
     config.available_features.add('matrix-fp16') # XMX implies the support of FP16 matrix
 
 if lit_config.params.get('matrix-fp16', False):
     config.available_features.add('matrix-fp16')
 
-#support for LIT parameter ur_l0_debug<num>
-if lit_config.params.get('ur_l0_debug'):
-    config.ur_l0_debug = lit_config.params.get('ur_l0_debug')
-    lit_config.note("UR_L0_DEBUG: "+config.ur_l0_debug)
-
-#support for LIT parameter ur_l0_leaks_debug
-if lit_config.params.get('ur_l0_leaks_debug'):
-    config.ur_l0_leaks_debug = lit_config.params.get('ur_l0_leaks_debug')
-    lit_config.note("UR_L0_LEAKS_DEBUG: "+config.ur_l0_leaks_debug)
+# support for LIT parameter ur_l0_debug<num>
+if lit_config.params.get("ur_l0_debug"):
+    config.ur_l0_debug = lit_config.params.get("ur_l0_debug")
+    lit_config.note("UR_L0_DEBUG: " + config.ur_l0_debug)
+
+# support for LIT parameter ur_l0_leaks_debug
+if lit_config.params.get("ur_l0_leaks_debug"):
+    config.ur_l0_leaks_debug = lit_config.params.get("ur_l0_leaks_debug")
+    lit_config.note("UR_L0_LEAKS_DEBUG: " + config.ur_l0_leaks_debug)
 
 # Make sure that any dynamic checks below are done in the build directory and
 # not where the sources are located. This is important for the in-tree
 # configuration (as opposite to the standalone one).
 os.chdir(config.sycl_obj_root)

@JackAKirk
Copy link
Contributor

JackAKirk commented Jan 8, 2024

Hi @steffenlarsen,

The release branch is failing with the following tests (I think on all backends, but definitely cuda and hip)

SYCL :: Basic/built-ins/vec_common.cpp
SYCL :: Basic/built-ins/vec_geometric.cpp
SYCL :: Basic/built-ins/vec_math.cpp
SYCL :: Basic/built-ins/vec_relational.cpp

These tests can be fixed by using the latest version of these tests in sycl tip, which could be achieved by cherry picking the following into the release branch (via this PR):

#11774
#11919
#11929

However, in particular since #11774 also includes runtime changes, I wanted to check with you whether you think these PRs should be cherry picked?

Thanks

@steffenlarsen
Copy link
Contributor

@JackAKirk - All the mentioned patches have been picked. I have not heard of any failures in those since that, so it may be CUDA/HIP specific.

@JackAKirk
Copy link
Contributor

@JackAKirk - All the mentioned patches have been picked. I have not heard of any failures in those since that, so it may be CUDA/HIP specific.

OK that's fine then thanks. They should make their way to sycl-rel-5_1_0 eventually I guess.

Add new 'are_graphs_supported' helper function to determine platform
support for Graphs
at runtime rather than the lit '// REQUIRES: <platform>' check at
compile time.

Fixed a bug in the usm_variant tests of trying to instantiate an int
with a float value.

This PR has been create in order to simplify the PR for adding OpenCL
support to the SYCL Graph tests
#11718.

---------

Co-authored-by: Ewan Crawford <ewan@codeplay.com>
…12432)

Calling urAdapterGet is required for the global adapter to be
initialized, but it was missing when creating a platform from a native
handle. This meant that the adapter's state never got initialized and
its refcount remained at 0 (and overflowed on teardown).

This had no visible side-effects until changes in #12403.
nrspruit and others added 2 commits January 22, 2024 13:51
Signed-off-by: Spruit, Neil R <neil.r.spruit@intel.com>
Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
UR testing.

Depends on #12375

---------

Co-authored-by: Kenneth Benzie (Benie) <k.benzie83@gmail.com>
oneapi-src/unified-runtime#1259

---------

Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
@kbenzie
Copy link
Contributor Author

kbenzie commented Jan 22, 2024

This CMake error is I believe due to changes in HIP 6 but this branch doesn't support that HIP version.

@kbenzie kbenzie closed this Feb 21, 2024
@kbenzie kbenzie deleted the benie/ur-v0.8-3 branch February 21, 2024 14:50
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.