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] Remove REQUIRES: build-and-run-mode in bindless_images tests #16789

Merged
merged 4 commits into from
Feb 4, 2025

Conversation

ayylol
Copy link
Contributor

@ayylol ayylol commented Jan 27, 2025

As of #16725 tests can properly react to features that affect compilation on build-only mode (i.e., libraries or OS), additionally we can also mark if a test should only be built for a specific triple using the target-* features.

This pr removes the REQUIRES: build-and-run-mode directives from bindless_images tests. These tests will now be marked as unsupported in environments that cannot build the tests due to missing the Vulkan library, or not being on Windows. For two tests removed the REQUIRES: build-and-run-mode directive by changing REQUIRES: cuda to REQUIRES: target-nvidia, this way the tests will only compile for nvidia triple.

@ayylol ayylol requested a review from a team as a code owner January 27, 2025 14:39
@ayylol
Copy link
Contributor Author

ayylol commented Jan 27, 2025

Note, some of the tests where I just removed the REQUIRES: build-and-run-mode may also need to have the REQUIRES: cuda replaced with REQUIRES: target-nvidia, I couldnt verify though because I dont have an environment readily available with vulkan. These changes are mainly meant to allow us to remove these exceptions and get a clean run on ci environment.

@ayylol
Copy link
Contributor Author

ayylol commented Jan 31, 2025

@intel/bindless-images-reviewers friendly ping

Copy link
Contributor

@przemektmalon przemektmalon left a comment

Choose a reason for hiding this comment

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

I've checked the vulkan_interop/ tests with // REQUIRES: target-nvidia and they pass. Feel free to apply the suggestions in this PR, or if you prefer, they can be made in another. Approved.

@ayylol
Copy link
Contributor Author

ayylol commented Jan 31, 2025

Ok, changed the vulkan tests to require target-nvidia instead

@ayylol
Copy link
Contributor Author

ayylol commented Feb 4, 2025

@intel/llvm-gatekeepers this is ready to merge, failure is the same as #16877

@dm-vodopyanov dm-vodopyanov merged commit 400fc6a into intel:sycl Feb 4, 2025
14 of 15 checks passed
@ayylol ayylol deleted the bindless-images-exceptions branch February 4, 2025 13:58
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