Skip to content

[ESIMD] Fix compilation from static libraries. #5826

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

Merged
merged 4 commits into from
Mar 18, 2022

Conversation

v-klochkov
Copy link
Contributor

@v-klochkov v-klochkov commented Mar 16, 2022

sycl-post-link previously removed "llvm.used" global, but that was
not enough as the initializer of "llvm.used" was not deleted and
some elements of initializer used kernels.
As a result the GenXSPIRVWriterAdaptor did not transform the kernel
arguments properly thinking that the kernel is called from another kernel,
which was not true.
Removing the initializer fixed the issue.

Signed-off-by: Vyacheslav N Klochkov [email protected]

sycl-post-link previously removed "llvm.used" global, but that was
not enough as the initializer of "llvm.used" was not deleted and
some elements of initializer used kernels.
As a result the GenXSPIRVWriterAdaptor did not transform the kernel
arguments properly thinking that the kernel was called from inside the module,
which was not true.
Removing the initializer fixed the issue.

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
@v-klochkov v-klochkov reopened this Mar 17, 2022
@v-klochkov v-klochkov force-pushed the esimd_static_linking_fix branch from ce155b8 to c2b9568 Compare March 17, 2022 00:47
kbobrovs
kbobrovs previously approved these changes Mar 17, 2022
Copy link
Contributor

@kbobrovs kbobrovs left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@v-klochkov
Copy link
Contributor Author

@dpcpp-tools-reviewers (@AlexeySachkov , @mlychkov ) - would you please take a look at this fix.
It fixes an error showing up only when dynamic library is built from static library, which is critically important scenario for some of our customers.

@mlychkov mlychkov requested a review from a team March 17, 2022 07:27
@mlychkov
Copy link
Contributor

Also may a test be included?

@v-klochkov
Copy link
Contributor Author

Also may a test be included?

Sure, the test is available here: intel/llvm-test-suite#923

Copy link
Contributor

@mlychkov mlychkov left a comment

Choose a reason for hiding this comment

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

Thank you very much.

@v-klochkov
Copy link
Contributor Author

/verify with intel/llvm-test-suite#923

@v-klochkov
Copy link
Contributor Author

13 CI checks passed, 1 CI check shows up as pending, but it also passed.
Also, this fix may affect only 1 test/scenario, that passed: SYCL/ESIMD/regression/complex-lib-lin.cpp
I am proceeding with the merge.

@v-klochkov v-klochkov merged commit e25f199 into intel:sycl Mar 18, 2022
@v-klochkov v-klochkov deleted the esimd_static_linking_fix branch March 18, 2022 17:57
v-klochkov added a commit to v-klochkov/llvm-test-suite that referenced this pull request Mar 22, 2022
The corresponding fix in the compiler: intel/llvm#5826

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
vladimirlaz pushed a commit to intel/llvm-test-suite that referenced this pull request Mar 24, 2022
…ary (#923)

The corresponding fix in the compiler: intel/llvm#5826

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
…ary (intel/llvm-test-suite#923)

The corresponding fix in the compiler: intel#5826

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
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.

3 participants