-
Notifications
You must be signed in to change notification settings - Fork 796
[SYCL][E2E] Fix interop_all_backends for CUDA #17643
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
Conversation
The CUDA backend has two different versions of the interop. A legacy version and a newer experimental version. Using the experimental version requires defining a macro before including the SYCL headers. This test uses the experimental version so it should define the macro.
2634102 to
bdb6997
Compare
HPS-1
left a comment
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.
LGTM
|
friendly ping @intel/llvm-reviewers-runtime @slawekptak |
|
@intel/llvm-reviewers-runtime @slawekptak could I get a review on this, it's a very small test update |
| // XFAIL: target-nvidia | ||
| // XFAIL-TRACKER: https://github.com/intel/llvm/issues/16070 | ||
| // RUN: %if target-spir %{ %{build} -o %t-opencl.out %} | ||
| // RUN: %if target-nvidia %{ %{build} -isystem %sycl_include -DBUILD_FOR_CUDA -o %t-cuda.out %} |
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.
Just to make sure, was the -isystem option removed intentionally?
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.
Yes, it's unnecessary, not sure why it was added in the first place, when it was added the test was already xfaild
|
@intel/llvm-gatekeepers this is ready to merge |
The CUDA backend has two different versions of the interop. A legacy version and a newer experimental version. Using the experimental version requires defining a macro before including the SYCL headers. This test uses the experimental version so it should define the macro.
Fixes: #16070