[SYCL][UR] Link UR PI against UR Loader #8637

bmyates · 2023-03-13T22:10:11Z

Compile UR L0 adapter as a standalone library

Signed-off-by: Brandon Yates brandon.yates@intel.com

sycl/plugins/unified_runtime/CMakeLists.txt

sycl/plugins/unified_runtime/pi2ur.hpp

sycl/plugins/unified_runtime/ur/adapters/level_zero/ur_level_zero.hpp

smaslov-intel · 2023-03-16T02:01:22Z

sycl/plugins/unified_runtime/ur/adapters/level_zero/ur_loader_interface.cpp

How is this new file used?

The UR loader calls these functions to build the DDI tables.

@bmyates this is the extra level of indirection that we dont want to have when there's only one adapter, right? that optimization could be done in a follow-up patch,.

No the extra level of indirection will be removed in the loader. No change to make to code here.

jandres742

+1

smaslov-intel

LGTM

sycl/plugins/unified_runtime/CMakeLists.txt

jandres742 · 2023-03-21T05:04:08Z

@bader : do you know what could be happening with the CI here?

Compile UR L0 adapter as a standalone library Signed-off-by: Brandon Yates <brandon.yates@intel.com>

bmyates · 2023-03-21T20:02:05Z

@bader : do you know what could be happening with the CI here?

CI works perfectly fine. This is a bug in CMake files. It should be fixed by c39de71. Please, update your branch.

I rebased and fixed that issue, but now failing with another issue that seems unrelated. Any idea @bader
/usr/bin/ld: /tmp/lit-tmp-8505fbty/level_zero_imm_cmdlist_per_thread-d70b3a.o: undefined reference to symbol 'pthread_create@@GLIBC_2.2.5

bader · 2023-03-21T20:06:37Z

@bader : do you know what could be happening with the CI here?

CI works perfectly fine. This is a bug in CMake files. It should be fixed by c39de71. Please, update your branch.

I rebased and fixed that issue, but now failing with another issue that seems unrelated. Any idea @bader /usr/bin/ld: /tmp/lit-tmp-8505fbty/level_zero_imm_cmdlist_per_thread-d70b3a.o: undefined reference to symbol 'pthread_create@@GLIBC_2.2.5

No idea.

bmyates · 2023-03-22T13:24:52Z

@bader : do you know what could be happening with the CI here?

CI works perfectly fine. This is a bug in CMake files. It should be fixed by c39de71. Please, update your branch.

I rebased and fixed that issue, but now failing with another issue that seems unrelated. Any idea @bader /usr/bin/ld: /tmp/lit-tmp-8505fbty/level_zero_imm_cmdlist_per_thread-d70b3a.o: undefined reference to symbol 'pthread_create@@GLIBC_2.2.5

This was caused by a bug in sycl tests that was fixed yesterday intel/llvm-test-suite#1674

Retriggered CI here, fingers crossed it passes this time.

bmyates · 2023-03-22T16:18:20Z

@smaslov-intel @bader all checks finally passed, needs reapproval now

bader

Minor style comments.

sycl/plugins/unified_runtime/ur/adapters/level_zero/ur_level_zero.cpp

sycl/plugins/unified_runtime/ur/adapters/level_zero/ur_loader_interface.cpp

sycl/plugins/unified_runtime/CMakeLists.txt

sycl/plugins/unified_runtime/ur/adapters/level_zero/ur_loader_interface.cpp

Signed-off-by: Brandon Yates <brandon.yates@intel.com>

smaslov-intel · 2023-03-23T21:22:36Z

@bmyates : we expect a clean CI run, unless there is something known that @intel/llvm-gatekeepers would be aware of.
All failures need to be tracked (with either a PR to temporarily disabling failed tests, or a PR that fixes the test or compiler)
@intel/llvm-gatekeepers, is this a known issue:

pi_die: Unknown parameter 4533 passed to hip_piKernelGetGroupInfo

bader · 2023-03-23T23:48:19Z

@bmyates : we expect a clean CI run, unless there is something known that @intel/llvm-gatekeepers would be aware of. All failures need to be tracked (with either a PR to temporarily disabling failed tests, or a PR that fixes the test or compiler)

👍
That's right. In case of PR disabling failed test, we also expect an issue to re-enable the test back.

@intel/llvm-gatekeepers, is this a known issue:

pi_die: Unknown parameter 4533 passed to hip_piKernelGetGroupInfo

I think @bmyates is right and this is a regression introduced by intel/llvm-test-suite@a92d98e. I see similar failure in #8708 pre-commits.

bader · 2023-03-24T04:11:35Z

@bmyates, could you fix, please?

/home/runner/work/llvm/llvm/src/sycl/plugins/unified_runtime/ur/adapters/level_zero/ur_level_zero.cpp:1745:43: error: unused parameter 'device_flags' [-Werror,-Wunused-parameter]
ur_result_t urInit(ur_device_init_flags_t device_flags) {
                                          ^
/home/runner/work/llvm/llvm/src/sycl/plugins/unified_runtime/ur/adapters/level_zero/ur_level_zero.cpp:1749:30: error: unused parameter 'pParams' [-Werror,-Wunused-parameter]
ur_result_t urTearDown(void *pParams) { return UR_RESULT_SUCCESS; }
                             ^
2 errors generated.

This reverts commit a0d0942.

Reverts #8637 It caused at least 3 known regressions. 1. #8637 (comment) 2. \sycl\plugins\unified_runtime\ur\adapters\level_zero\ur_level_zero_common.hpp(10): fatal error C1083: Cannot open include file: 'sycl/detail/pi.h': No such file or directory 3. ld: error: undefined symbol: pthread_rwlock_wrlock (with GCC 7.5)

Compile UR L0 adapter as a standalone library Signed-off-by: Brandon Yates <brandon.yates@intel.com>

Reverts intel#8637 It caused at least 3 known regressions. 1. intel#8637 (comment) 2. \sycl\plugins\unified_runtime\ur\adapters\level_zero\ur_level_zero_common.hpp(10): fatal error C1083: Cannot open include file: 'sycl/detail/pi.h': No such file or directory 3. ld: error: undefined symbol: pthread_rwlock_wrlock (with GCC 7.5)

Reverts intel/llvm#8637 It caused at least 3 known regressions. 1. intel/llvm#8637 (comment) 2. \sycl\plugins\unified_runtime\ur\adapters\level_zero\ur_level_zero_common.hpp(10): fatal error C1083: Cannot open include file: 'sycl/detail/pi.h': No such file or directory 3. ld: error: undefined symbol: pthread_rwlock_wrlock (with GCC 7.5)

bmyates requested a review from a team as a code owner March 13, 2023 22:10

bmyates requested a review from dm-vodopyanov March 13, 2023 22:10

bmyates mentioned this pull request Mar 13, 2023

[SYCL][UR] Add support ur_loader #8423

Closed

bmyates temporarily deployed to aws March 13, 2023 22:38 — with GitHub Actions Inactive

smaslov-intel reviewed Mar 13, 2023

View reviewed changes

sycl/plugins/unified_runtime/CMakeLists.txt Show resolved Hide resolved

bmyates temporarily deployed to aws March 13, 2023 23:28 — with GitHub Actions Inactive

igchor reviewed Mar 13, 2023

View reviewed changes

sycl/plugins/unified_runtime/CMakeLists.txt Outdated Show resolved Hide resolved

sycl/plugins/unified_runtime/CMakeLists.txt Outdated Show resolved Hide resolved

bmyates temporarily deployed to aws March 15, 2023 01:03 — with GitHub Actions Inactive

bmyates requested a review from a team as a code owner March 16, 2023 01:02

bmyates temporarily deployed to aws March 16, 2023 01:11 — with GitHub Actions Inactive

smaslov-intel reviewed Mar 16, 2023

View reviewed changes

sycl/plugins/unified_runtime/pi2ur.hpp Show resolved Hide resolved

smaslov-intel reviewed Mar 16, 2023

View reviewed changes

sycl/plugins/unified_runtime/ur/adapters/level_zero/ur_level_zero.hpp Outdated Show resolved Hide resolved

smaslov-intel reviewed Mar 16, 2023

View reviewed changes

bmyates force-pushed the use_loader branch from 98e4e83 to eb2a670 Compare March 16, 2023 16:32

jandres742 reviewed Mar 16, 2023

View reviewed changes

bmyates temporarily deployed to aws March 16, 2023 17:04 — with GitHub Actions Inactive

smaslov-intel approved these changes Mar 16, 2023

View reviewed changes

bmyates temporarily deployed to aws March 16, 2023 17:45 — with GitHub Actions Inactive

bmyates temporarily deployed to aws March 16, 2023 23:29 — with GitHub Actions Inactive

bmyates temporarily deployed to aws March 17, 2023 00:01 — with GitHub Actions Inactive

smaslov-intel approved these changes Mar 17, 2023

View reviewed changes

sycl/plugins/unified_runtime/CMakeLists.txt Outdated Show resolved Hide resolved

bmyates force-pushed the use_loader branch from 48ff976 to ffa80e4 Compare March 17, 2023 19:40

smaslov-intel approved these changes Mar 18, 2023

View reviewed changes

bmyates force-pushed the use_loader branch from ffa80e4 to 808869d Compare March 20, 2023 20:23

bmyates temporarily deployed to aws March 20, 2023 21:36 — with GitHub Actions Inactive

bmyates closed this Mar 20, 2023

bmyates reopened this Mar 20, 2023

bmyates temporarily deployed to aws March 20, 2023 22:42 — with GitHub Actions Inactive

[SYCL][UR] Link UR PI against UR Loader

a274ec9

Compile UR L0 adapter as a standalone library Signed-off-by: Brandon Yates <brandon.yates@intel.com>

bmyates temporarily deployed to aws March 21, 2023 15:46 — with GitHub Actions Inactive

bmyates temporarily deployed to aws March 21, 2023 17:38 — with GitHub Actions Inactive

bmyates closed this Mar 22, 2023

bmyates reopened this Mar 22, 2023

bmyates temporarily deployed to aws March 22, 2023 14:15 — with GitHub Actions Inactive

bmyates temporarily deployed to aws March 22, 2023 15:45 — with GitHub Actions Inactive

bader requested a review from smaslov-intel March 22, 2023 22:17

smaslov-intel approved these changes Mar 22, 2023

View reviewed changes

bader approved these changes Mar 22, 2023

View reviewed changes

format fixes

aae13a8

Signed-off-by: Brandon Yates <brandon.yates@intel.com>

bmyates temporarily deployed to aws March 23, 2023 19:39 — with GitHub Actions Inactive

bmyates temporarily deployed to aws March 23, 2023 22:17 — with GitHub Actions Inactive

bader merged commit a0d0942 into intel:sycl Mar 24, 2023

jandres742 mentioned this pull request Mar 24, 2023

[SYCL-MLIR] Merge from intel/llvm sycl branch #8774

Closed

whitneywhtsang added a commit to whitneywhtsang/llvm that referenced this pull request Mar 25, 2023

Revert "[SYCL][UR] Link UR PI against UR Loader (intel#8637)"

f372b39

This reverts commit a0d0942.

bader added a commit that referenced this pull request Mar 27, 2023

Revert "[SYCL][UR] Link UR PI against UR Loader (#8637)"

356ad2b

This reverts commit a0d0942.

bader mentioned this pull request Mar 27, 2023

Revert "[SYCL][UR] Link UR PI against UR Loader" #8781

Merged

veselypeta pushed a commit to veselypeta/llvm that referenced this pull request Sep 21, 2023

[SYCL][UR] Link UR PI against UR Loader (intel#8637)

3038a27

Compile UR L0 adapter as a standalone library Signed-off-by: Brandon Yates <brandon.yates@intel.com>

[SYCL][UR] Link UR PI against UR Loader #8637

[SYCL][UR] Link UR PI against UR Loader #8637

Uh oh!

Conversation

bmyates commented Mar 13, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smaslov-intel Mar 16, 2023

Choose a reason for hiding this comment

Uh oh!

bmyates Mar 16, 2023

Choose a reason for hiding this comment

Uh oh!

jandres742 Mar 16, 2023

Choose a reason for hiding this comment

Uh oh!

bmyates Mar 16, 2023

Choose a reason for hiding this comment

Uh oh!

jandres742 left a comment

Choose a reason for hiding this comment

Uh oh!

smaslov-intel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jandres742 commented Mar 21, 2023

Uh oh!

bmyates commented Mar 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bader commented Mar 21, 2023

Uh oh!

bmyates commented Mar 22, 2023

Uh oh!

bmyates commented Mar 22, 2023

Uh oh!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smaslov-intel commented Mar 23, 2023

Uh oh!

bader commented Mar 23, 2023

Uh oh!

bader commented Mar 24, 2023

Uh oh!

Uh oh!

bmyates commented Mar 21, 2023 •

edited

Loading