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.

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

[SYCL][E2E] Re-enable interop_all_backends.cpp test #15767

Merged
merged 11 commits into from
Oct 25, 2024

Conversation

ayylol
Copy link
Contributor

@ayylol ayylol commented Oct 18, 2024

The capitalization of cuda and hip in Basic/interop/interop_all_backends.cpp makes the test always unsupported. This patch removes the requires line, and refactors how the test is built so that we build an executable for each backend.

@ayylol ayylol changed the title [SYCL][E2E] Fix typos in REQUIRES: lit lines [SYCL][E2E] Fix capitalization in REQUIRES: lit line Oct 21, 2024
// REQUIRES: CUDA || HIP
// RUN: %{build} %if hip %{ -DSYCL_EXT_ONEAPI_BACKEND_HIP %} %else %{ %if cuda %{ -DSYCL_EXT_ONEAPI_BACKEND_CUDA_EXPERIMENTAL %} %else %{ %if level_zero %{ -DSYCL_EXT_ONEAPI_BACKEND_L0 %} %} %} -o %t.out
// REQUIRES: cuda || hip
// RUN: %{build} %if any-device-is-hip %{ -DSYCL_EXT_ONEAPI_BACKEND_HIP %} %else %{ %if any-device-is-cuda %{ -DSYCL_EXT_ONEAPI_BACKEND_CUDA_EXPERIMENTAL %} %else %{ %if any-device-is-level_zero %{ -DSYCL_EXT_ONEAPI_BACKEND_L0 %} %} %} -o %t.out
Copy link
Contributor

@againull againull Oct 21, 2024

Choose a reason for hiding this comment

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

This way we will build an executable only for hip, or only for cuda, or only for l0.
Probably this line should be split into three lines with different executable names.

@ayylol ayylol changed the title [SYCL][E2E] Fix capitalization in REQUIRES: lit line [SYCL][E2E] Re-enableBasic/interop/interop_all_backends.cpp test Oct 22, 2024
@ayylol ayylol changed the title [SYCL][E2E] Re-enableBasic/interop/interop_all_backends.cpp test [SYCL][E2E] Re-enable interop_all_backends.cpp test Oct 22, 2024
@againull againull merged commit 7e2d615 into intel:sycl Oct 25, 2024
13 checks passed
@ayylol ayylol deleted the requires-typo branch October 25, 2024 17:59
@sarnex
Copy link
Contributor

sarnex commented Oct 25, 2024

@ayylol Test is failing on HIP in postcommit unfortunately. Think you can fix it quickly? If not let's revert for now

https://github.com/intel/llvm/actions/runs/11523103039/job/32080880588

-- Testing: 2233 tests, 24 workers --
FAIL: SYCL :: Basic/interop/interop_all_backends.cpp (278 of 2233)
******************** TEST 'SYCL :: Basic/interop/interop_all_backends.cpp' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 3 has no command after substitutions
# RUN: at line 4 has no command after substitutions
# RUN: at line 5 has no command after substitutions
# RUN: at line 6
/__w/llvm/llvm/toolchain/bin//clang++  -Werror -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx1031 -fsycl -fsycl-targets=amdgcn-amd-amdhsa  /__w/llvm/llvm/llvm/sycl/test-e2e/Basic/interop/interop_all_backends.cpp -DBUILD_FOR_HIP -o /__w/llvm/llvm/build-e2e/Basic/interop/Output/interop_all_backends.cpp.tmp-hip.out
# executed command: /__w/llvm/llvm/toolchain/bin//clang++ -Werror -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx1031 -fsycl -fsycl-targets=amdgcn-amd-amdhsa /__w/llvm/llvm/llvm/sycl/test-e2e/Basic/interop/interop_all_backends.cpp -DBUILD_FOR_HIP -o /__w/llvm/llvm/build-e2e/Basic/interop/Output/interop_all_backends.cpp.tmp-hip.out
# .---command stderr------------
# | /__w/llvm/llvm/llvm/sycl/test-e2e/Basic/interop/interop_all_backends.cpp:23:22: error: unknown type name 'hipDevice_t'
# |    23 | using nativeDevice = hipDevice_t;
# |       |                      ^
# | /__w/llvm/llvm/llvm/sycl/test-e2e/Basic/interop/interop_all_backends.cpp:24:21: error: unknown type name 'hipStream_t'; did you mean 'ihipStream_t'?
# |    24 | using nativeQueue = hipStream_t;
# |       |                     ^~~~~~~~~~~
# |       |                     ihipStream_t
# | /__w/llvm/llvm/toolchain/bin/../include/sycl/detail/backend_traits_hip.hpp:26:16: note: 'ihipStream_t' declared here
# |    26 | typedef struct ihipStream_t *HIPstream;
# |       |                ^
# | /__w/llvm/llvm/llvm/sycl/test-e2e/Basic/interop/interop_all_backends.cpp:25:21: error: unknown type name 'hipEvent_t'; did you mean 'ihipEvent_t'?
# |    25 | using nativeEvent = hipEvent_t;
# |       |                     ^~~~~~~~~~
# |       |                     ihipEvent_t
# | /__w/llvm/llvm/toolchain/bin/../include/sycl/detail/backend_traits_hip.hpp:27:16: note: 'ihipEvent_t' declared here
# |    27 | typedef struct ihipEvent_t *HIPevent;
# |       |                ^
# | /__w/llvm/llvm/llvm/sycl/test-e2e/Basic/interop/interop_all_backends.cpp:45:22: error: use of undeclared identifier 'nativeDevice'
# |    45 |                      nativeDevice>));
# |       |                      ^
# | 4 errors generated.
# `-----------------------------
# error: command failed with exit status: 1

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