Skip to content

[SYCL] Fix zstd build when static zstd libraries are not found #15856

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 24, 2024
Merged

[SYCL] Fix zstd build when static zstd libraries are not found #15856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
938b3e6
Fix zstd build when static zstd libraries are not avilable
uditagarwal97 Oct 24, 2024
cdc936e
Merge branch 'sycl' into fix_zstd_build
uditagarwal97 Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions buildbot/configure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def do_configure(args):

sycl_enable_xpti_tracing = "ON"
xpti_enable_werror = "OFF"
llvm_enable_zstd = "OFF"
llvm_enable_zstd = "ON"

if sys.platform != "darwin":
sycl_enabled_backends.append("level_zero")
Expand Down Expand Up @@ -134,8 +134,6 @@ def do_configure(args):

# For clang-format, clang-tidy and code coverage
llvm_enable_projects += ";clang-tools-extra;compiler-rt"
# Build with zstd disabled on CI for now.
llvm_enable_zstd = "OFF"
if sys.platform != "darwin":
# libclc is required for CI validation
libclc_enabled = True
Expand Down
7 changes: 7 additions & 0 deletions llvm/cmake/config-ix.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,14 @@ if(LLVM_ENABLE_ZSTD)
find_package(zstd QUIET)
endif()
endif()

# If LLVM_USE_STATIC_ZSTD is specified, make sure we enable zstd only if static
# libraries are found.
Copy link
Contributor Author

@uditagarwal97 uditagarwal97 Oct 24, 2024

Choose a reason for hiding this comment

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

I'll create a bug report and PR to upstream this change, if approved.

Copy link
Contributor Author

@uditagarwal97 uditagarwal97 Oct 24, 2024

Choose a reason for hiding this comment

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

For upstreaming:
issue: llvm/llvm-project#113583
PR: llvm/llvm-project#113584

if(LLVM_USE_STATIC_ZSTD AND NOT TARGET zstd::libzstd_static)
Copy link
Contributor

@sarnex sarnex Oct 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

sorry what is zstd::libzstd_static and who sets it? is it set by find_package if it finds the static lib? not a cmake expert

Copy link
Contributor Author

@uditagarwal97 uditagarwal97 Oct 24, 2024

Choose a reason for hiding this comment

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

Yes, zstd::libzstd_static is set during find_package if it finds static ZSTD libraries. https://github.com/intel/llvm/blob/sycl/llvm/cmake/modules/Findzstd.cmake#L60

set(LLVM_ENABLE_ZSTD OFF)
else()
set(LLVM_ENABLE_ZSTD ${zstd_FOUND})
endif()

if(LLVM_ENABLE_LIBXML2)
if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON)
Expand Down
Loading