Skip to content
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

[CI] Enable check-sycl-unittests in CI #12858

Merged
merged 25 commits into from
Mar 21, 2024
Merged

[CI] Enable check-sycl-unittests in CI #12858

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2294de7
Enable check-sycl-unittests in CI
uditagarwal97 Feb 28, 2024
3c94b43
Print debug stats
uditagarwal97 Feb 29, 2024
3428ee3
Install
uditagarwal97 Feb 29, 2024
f7c1fa5
Add to env vars
uditagarwal97 Feb 29, 2024
e7e93e4
Fix
uditagarwal97 Mar 4, 2024
4141f31
Disable failing unittest
uditagarwal97 Mar 5, 2024
cdd8d73
Enable on windows.
uditagarwal97 Mar 5, 2024
bbe4266
Fix typo
uditagarwal97 Mar 5, 2024
520d645
fix
uditagarwal97 Mar 7, 2024
c28e980
Add debug info
uditagarwal97 Mar 7, 2024
1015f8f
fix
uditagarwal97 Mar 8, 2024
ae7c754
Third time's the charm
uditagarwal97 Mar 8, 2024
cfab312
Fourth time sucks!
uditagarwal97 Mar 8, 2024
691616b
print debug info
uditagarwal97 Mar 8, 2024
001d567
Fix formatting
uditagarwal97 Mar 8, 2024
85f388c
Cleanup
uditagarwal97 Mar 8, 2024
6c4e961
Update QueueIDCheck.cpp
uditagarwal97 Mar 11, 2024
c6b3e3b
Test moving to cmake file.
uditagarwal97 Mar 18, 2024
4f25c62
Fix Windows build
uditagarwal97 Mar 18, 2024
e9d598a
Try setting PATH env var
uditagarwal97 Mar 18, 2024
4663ead
try2
uditagarwal97 Mar 18, 2024
219bbfe
Another try
uditagarwal97 Mar 19, 2024
54e5c60
Cleanup
uditagarwal97 Mar 19, 2024
c0aa422
Another try
uditagarwal97 Mar 20, 2024
c50a6fc
Finalize
uditagarwal97 Mar 20, 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
9 changes: 9 additions & 0 deletions .github/workflows/sycl-linux-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,15 @@ jobs:
# TODO consider moving this to Dockerfile.
export LD_LIBRARY_PATH=/usr/local/cuda/compat/:/usr/local/cuda/lib64:$LD_LIBRARY_PATH
cmake --build $GITHUB_WORKSPACE/build --target check-sycl
- name: check-sycl-unittests
if: always() && !cancelled() && contains(inputs.changes, 'sycl')
run: |
# TODO consider moving this to Dockerfile.
export LD_LIBRARY_PATH=/usr/local/cuda/compat/:/usr/local/cuda/lib64:$LD_LIBRARY_PATH

export LD_LIBRARY_PATH=$GITHUB_WORKSPACE/build/lib:$LD_LIBRARY_PATH
export PATH=$GITHUB_WORKSPACE/build/bin:$PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a comment explaining the issue and why we deal with it this way and what other alternatives were rejected.

cmake --build $GITHUB_WORKSPACE/build --target check-sycl-unittests
- name: check-llvm-spirv
if: always() && !cancelled() && contains(inputs.changes, 'llvm_spirv')
run: |
Expand Down
8 changes: 8 additions & 0 deletions .github/workflows/sycl-windows-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ jobs:
shell: bash
run: |
cmake --build build --target sycl-toolchain
- name: Setup SYCL toolchain
run: |
# Need this for check-sycl-unittests.
echo "PATH=$env:GITHUB_WORKSPACE\\build\\bin;$env:PATH" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append
Copy link
Contributor

Choose a reason for hiding this comment

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

Why here and not in cmake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did tried moving this to the CMake file, but that doesn't seem to help. Setting PATH in AddSYCLUnitTest.cmake file seems to reflect in the "cmake -E" command but the updated PATH is not visible when the unit tests are actually executed. My hypothesis is CMake is spawning two subprocess, one for compiling (cmake -E command) and another for running the unit tests. Updating PATH in the cmake file seems to affect only the former subprocess.

Nevertheless, this problem of check-sycl-unittests using old sycl library is only affecting CI, so just adding a workaround in the workflow file seems to suffice for the time being.

Copy link
Contributor

Choose a reason for hiding this comment

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

using old sycl library is only affecting CI

I don't think it's true. It would be the same locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not able to reproduce this locally, both on Linux and Windows (with and without u4win).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, you are right - I'm now able to reproduce it locally. This time, I kept two intel/llvm installations and before running ninja check-sycl-unittests in one, I explicitly added the sycl DLL of the second installation to the PATH.

- name: check-llvm
if: always() && !cancelled() && contains(inputs.changes, 'llvm')
run: |
Expand All @@ -109,6 +113,10 @@ jobs:
if: always() && !cancelled() && contains(inputs.changes, 'sycl')
run: |
cmake --build build --target check-sycl
- name: check-sycl-unittests
if: always() && !cancelled() && contains(inputs.changes, 'sycl')
run: |
cmake --build build --target check-sycl-unittests
- name: check-llvm-spirv
if: always() && !cancelled() && contains(inputs.changes, 'llvm_spirv')
run: |
Expand Down
8 changes: 4 additions & 4 deletions sycl/cmake/modules/AddSYCLUnitTest.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ macro(add_sycl_unittest test_dirname link_variant)
add_custom_target(check-sycl-${test_dirname}
${CMAKE_COMMAND} -E env
LLVM_PROFILE_FILE="${SYCL_COVERAGE_PATH}/${test_dirname}.profraw"
env SYCL_CONFIG_FILE_NAME=null.cfg
env SYCL_DEVICELIB_NO_FALLBACK=1
env SYCL_CACHE_DIR="${CMAKE_BINARY_DIR}/sycl_cache"
SYCL_CONFIG_FILE_NAME=null.cfg
SYCL_DEVICELIB_NO_FALLBACK=1
SYCL_CACHE_DIR="${CMAKE_BINARY_DIR}/sycl_cache"
${CMAKE_CURRENT_BINARY_DIR}/${test_dirname}
DEPENDS
${test_dirname}
Expand All @@ -60,7 +60,7 @@ macro(add_sycl_unittest test_dirname link_variant)
if(SYCL_ENABLE_KERNEL_FUSION)
target_link_libraries(${test_dirname} PRIVATE sycl-fusion)
endif(SYCL_ENABLE_KERNEL_FUSION)

target_include_directories(${test_dirname}
PRIVATE SYSTEM
${sycl_inc_dir}
Expand Down
2 changes: 1 addition & 1 deletion sycl/unittests/xpti_trace/QueueIDCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ TEST_F(QueueID, QueueCreationAndKernelWithDeps) {
checkTaskBeginEnd(QueueIDSTr);
}

TEST_F(QueueID, QueueCreationUSMOperations) {
TEST_F(QueueID, DISABLED_QueueCreationUSMOperations) {
aelovikov-intel marked this conversation as resolved.
Show resolved Hide resolved
sycl::queue Q0;
auto Queue0ImplPtr = sycl::detail::getSyclObjImpl(Q0);
auto QueueIDSTr = std::to_string(Queue0ImplPtr->getQueueID());
Expand Down
Loading