Skip to content

[SYCL][CPU] Fix deployment of opencl-aot tool #949

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

Jump to bottom
Merged
merged 5 commits into from
Dec 20, 2019
Merged

[SYCL][CPU] Fix deployment of opencl-aot tool #949

merged 5 commits into from
Dec 20, 2019

Conversation

@dm-vodopyanov
Copy link
Contributor

@dm-vodopyanov dm-vodopyanov commented Dec 18, 2019 •
edited
Loading

opencl-aot didn't copy to install folder. Added deploy-opencl-aot target to fix this.

Signed-off-by: Dmitry Vodopyanov [email protected]

@vladimirlaz
Copy link
Contributor

vladimirlaz commented Dec 18, 2019

Code looks good will approve once tests finish

@vladimirlaz
Copy link
Contributor

vladimirlaz commented Dec 18, 2019

Looks like OpenCL ICD oader is still installed as part of opencl-aot:
-- Install configuration: "Release"
-- Installing: /localdisk2/sys_bbsycl/buildbot/worker/sycl-ubu-x64-pr/llvm.obj/tools/opencl-aot/lib/libOpenCL.so.1.2
-- Installing: /localdisk2/sys_bbsycl/buildbot/worker/sycl-ubu-x64-pr/llvm.obj/tools/opencl-aot/lib/libOpenCL.so.1
-- Installing: /localdisk2/sys_bbsycl/buildbot/worker/sycl-ubu-x64-pr/llvm.obj/tools/opencl-aot/lib/libOpenCL.so

@dm-vodopyanov
Copy link
Contributor Author

dm-vodopyanov commented Dec 18, 2019

@vladimirlaz these files don't deploy to .../llvm.obj/install folder, so thy are not part of the final syclos build. You can also see builds earlier than 12/17/19 - they also have lib64 folder with these files in it.
The root cause is dependency step, which hasn't been modified as part of opencl-aot work:

-- Install configuration: ""
-- Installing: /localdisk2/sys_bbsycl/buildbot/worker/sycl-ubu-x64-pr/llvm.obj/install/lib/libOpenCL.so.1.2
-- Installing: /localdisk2/sys_bbsycl/buildbot/worker/sycl-ubu-x64-pr/llvm.obj/install/lib/libOpenCL.so.1
-- Installing: /localdisk2/sys_bbsycl/buildbot/worker/sycl-ubu-x64-pr/llvm.obj/install/lib/libOpenCL.so

@dm-vodopyanov
Copy link
Contributor Author

dm-vodopyanov commented Dec 18, 2019 •
edited
Loading

@vladimirlaz please don't merge it.
check-sycl on Windows is ok now:
Found AOT device compiler opencl-aot
but it's still lost on Linux, though it was installed correctly now:
check-sycl: Could not find AOT device compiler opencl-aot
compile:

[3214/3215] Deploying component opencl-aot
-- Install configuration: "Release"
-- Installing: /localdisk2/sys_bbsycl/buildbot/worker/sycl-ubu-x64-pr/llvm.obj/install/bin/opencl-aot```

@vladimirlaz
Copy link
Contributor

vladimirlaz commented Dec 19, 2019 •
edited
Loading

@dm-vodopyanov From my understanding the follwoing line means that the libraries are installed
-- Installing: /localdisk2/sys_bbsycl/buildbot/worker/sycl-ubu-x64-pr/llvm.obj/install/lib/libOpenCL.so.1.2

Could you make sure that it does not happen.

For LIT tests: please make sure that you add the new component to dependency list of LIT tests to make sure it is built before LIT test are run (otherwise it may be built in parallel thread while LIT tests are checking for tool presence). But it should be "optional" dependency. If it is missed (remove corresponding project from CMake config) build should not be blocked (but the the file should be missed in LIT testing).

vladimirlaz
vladimirlaz suggested changes Dec 20, 2019
View reviewed changes
@dm-vodopyanov
[SYCL][CPU] Update opencl-aot/CMakeLists.txt - if OpenCL_INCLUDE_DIR …
494a076 
…AND OpenCL_LIBRARY set, there is no extra downloading of OpenCL headers and ICD loader

Signed-off-by: Dmitry Vodopyanov <[email protected]>
vladimirlaz
vladimirlaz reviewed Dec 20, 2019
View reviewed changes
@vladimirlaz vladimirlaz merged commit 4d3f536 into intel:sycl Dec 20, 2019
bader
bader reviewed Dec 20, 2019
View reviewed changes
opencl-aot/CMakeLists.txt
COMMENT "Downloading OpenCL headers."
)
else ()
message("Copying OpenCL headers...")
Copy link
Contributor

@bader bader Dec 20, 2019

Choose a reason for hiding this comment

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

What is this for?

Copy link
Contributor Author

@dm-vodopyanov dm-vodopyanov Dec 20, 2019

Choose a reason for hiding this comment

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

This is for notification about copying headers/ICD on configure step. Should it be removed?

Copy link
Contributor

@bader bader Dec 20, 2019

Choose a reason for hiding this comment

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

I think it's confusing. If I understand correctly headers are not copied during build files configuration. It looks like some debug messages...
I think it's better to remove or rephrase.

Copy link
Contributor Author

@dm-vodopyanov dm-vodopyanov Dec 20, 2019

Choose a reason for hiding this comment

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

Created PR #959 with fix

aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Feb 23, 2023
@vladimirlaz
[SYCL][XPTI] Added test covering kernel info (intel#949)
5a03ed6 
 - fix BE selection for buffer tests
 - kernel execution parameter (global size, local size, offset)
 - kernel argument parameters (number, size, pointer which can be
associated with accessor/USM buffer)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bader bader bader left review comments

+1 more reviewer

@vladimirlaz vladimirlaz vladimirlaz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@vladimirlaz vladimirlaz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dm-vodopyanov @vladimirlaz @bader