Skip to content

[UR] Update UR loader tag to fix libsycl self-build on windows#19851

Merged
dm-vodopyanov merged 6 commits into
intel:syclfrom
KseniyaTikhomirova:update_ur
Aug 28, 2025
Merged

[UR] Update UR loader tag to fix libsycl self-build on windows#19851
dm-vodopyanov merged 6 commits into
intel:syclfrom
KseniyaTikhomirova:update_ur

Conversation

@KseniyaTikhomirova

Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
# Remember to update the pkg_check_modules minimum version above when updating the
# clone tag
set(UR_LEVEL_ZERO_LOADER_TAG 35c037cdf4aa9a2e6df34b6f1ce1bdc86ac5422f)
set(UR_LEVEL_ZERO_LOADER_TAG 2e212f73c493bfab0e52381b175131a373db0ca5)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
set(UR_LEVEL_ZERO_LOADER_TAG 2e212f73c493bfab0e52381b175131a373db0ca5)
set(UR_LEVEL_ZERO_LOADER_TAG v1.24.1)

https://github.com/oneapi-src/level-zero/releases/tag/v1.24.1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh, that is not correct, my fix is merged later that 1.24.1 is created. how should I update pkg_check_modules properly then? I set the last version, but it seems to be not the right way

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hm, we'd need a new tag if we want those to match. @nrspruit ping.

But I don't think this failure: https://github.com/intel/llvm/actions/runs/17152112326/job/48661284629?pr=19851#step:8:112 is related to mismatch between this tag and the pkg config one. It looks like targets changed in level-zero, and we will need to change them as well in UR cmakes.

@KseniyaTikhomirova KseniyaTikhomirova Aug 22, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this UR commit caused this regression, it works fine if I revert it 8959a5e
maybe clean build has never been done after? my changes prevent usage of prebuilt l0 adapter version since I request a newer one
I've created draft PR with revert and update to double check: #19853

@pbalcer pbalcer Aug 22, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe clean build has never been done after? my changes prevent usage of prebuilt l0 adapter version since I request a newer one

Seems likely. 8959a5e#diff-e83af30e7502eb85108ac495d4a4d50775e6b0eba4e27e8b682c797f8b8a168fR67-R74 looking at this, it seems that we no longer install ze_loader target. @sarnex Is this intentional?

old:
install(TARGETS ur_umf LevelZeroLoader LevelZeroLoader-Headers ComputeRuntimeLevelZero-Headers ze_loader level_zero_utils

new:
set(ADAPTER_L0_TARGETS_TO_INSTALL ur_umf LevelZeroLoader LevelZeroLoader-Headers ComputeRuntimeLevelZero-Headers)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Merged it in 7b17ee3

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Works for me locally. The target is defined here.

I meant that I don't see "ze-loader" version.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah right, yeah it was a typo :P

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

L0 tag will be added soon oneapi-src/level-zero#369

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@KseniyaTikhomirova
KseniyaTikhomirova marked this pull request as ready for review August 22, 2025 13:52
@KseniyaTikhomirova
KseniyaTikhomirova requested a review from a team as a code owner August 22, 2025 13:52
sarnex added a commit that referenced this pull request Aug 27, 2025
See #19851

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>
Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
github-actions Bot pushed a commit to oneapi-src/unified-runtime that referenced this pull request Aug 28, 2025
See intel/llvm#19851

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>
@KseniyaTikhomirova

Copy link
Copy Markdown
Contributor Author

@intel/llvm-gatekeepers hi, could you please merge this PR?

@dm-vodopyanov
dm-vodopyanov merged commit ba85a64 into intel:sycl Aug 28, 2025
36 checks passed
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.

7 participants