-
Notifications
You must be signed in to change notification settings - Fork 794
[LLVM] Check if zstd static libraries are built with -fPIC flag
#15970
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
base: sycl
Are you sure you want to change the base?
Conversation
|
Once approved, I'll create a PR to upstream these changes. |
| set(LLVM_ENABLE_ZSTD ${zstd_FOUND}) | ||
| # Check if the zstd static library is usable when BUILD_SHARED_LIBS is ON | ||
| # Test linking zstd::libzstd_static with a shared library. This is to ensure | ||
| # that the static library is built with -fPIC flag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i havent seen this kind of issue before, is that because almost everything is built with -fPIC so it just works?
are there any other examples in llvm or other open source projects of solving it this way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i havent seen this kind of issue before, is that because almost everything is built with
-fPICso it just works?
This issue was observed when DPC++ is built in shared lib configuration on Ubuntu 24.04. Apparently, the zstd static library that comes with Ubuntu 24.04 was built without the -fPIC flag and thus it gives error when linking to dynamic libraries. I've filed a bug report about the zstd package in Ubuntu 24.04 here: https://bugs.launchpad.net/ubuntu/+source/libzstd/+bug/2086543
We did not observe this error before because the zstd package that we use in CI (Ubuntu 22.04) was built correctly with fPIC flag.
are there any other examples in llvm or other open source projects of solving it this way?
I'm not aware of any. Let me dig around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah my overalll question is i've never seen this kind of issue before so i don't know what the fix should look like so i can't give a good review :)
|
This pull request is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be automatically closed in 30 days. |
Fixes: #15935
Problem
DPC++ shared lib built fails when linking static
zstdlibraries that are not position-independent. See #15935 for more info.Proposed Solution
Since
zstdis optional to DPC++, we check ifzstdstatic libraries are built with-fPICflag or not. If not, we disablezstd.