Skip to content

[NFC][SYCL][Driver] Add missing msan device libraries for testing #16446

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

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

mdtoguchi
Copy link
Contributor

For driver testing, we rely on the existance of device libraries to satisfy the pulling in of required device libraries. When the compiler is built without the libraries, testing needs to use the specific test inputs to resolve.

Add the missing msan device libraries to fix failing tests when the device libraries are not built (e.g. sanitizer specific builds)

For driver testing, we rely on the existance of device libraries to
satisfy the pulling in of required device libraries.  When the compiler
is built without the libraries, testing needs to use the specific test
inputs to resolve.

Add the missing msan device libraries to fix failing tests when the
device libraries are not built (e.g. sanitizer specific builds)
@mdtoguchi mdtoguchi requested a review from a team as a code owner December 20, 2024 17:33
@bader
Copy link
Contributor

bader commented Dec 27, 2024

Can the test create such files in temporary directory instead of keeping these under version control?

@mdtoguchi
Copy link
Contributor Author

Can the test create such files in temporary directory instead of keeping these under version control?

@bader - I equate these items to be similar to system/gcc dependent items that are needed to satisfy environmental requirements. We have the general Inputs location as these dependent device libraries impact a few tests.

We could make it local to each individual test, but it would take a larger effort. Could this be pursued in a subsequent PR?

@bader
Copy link
Contributor

bader commented Jan 2, 2025

Can the test create such files in temporary directory instead of keeping these under version control?

@bader - I equate these items to be similar to system/gcc dependent items that are needed to satisfy environmental requirements. We have the general Inputs location as these dependent device libraries impact a few tests.

We could make it local to each individual test, but it would take a larger effort. Could this be pursued in a subsequent PR?

I see that some tests create files (e.g. clang/test/Driver/modules-print-library-module-manifest-path.cpp), but other tests re-use clang/test/Driver/Inputs/ files. I'm not sure what is the rule, but I guess if multiple tests rely on the same input files it makes sense to keep them in the tree. For a single test, it's probably better to create them on the fly.

If these files are supposed to be used by multiple tests, we should probably add them to clang/test/Driver/Inputs/ and ignore my comment.

@mdtoguchi
Copy link
Contributor Author

If these files are supposed to be used by multiple tests, we should probably add them to clang/test/Driver/Inputs/ and ignore my comment.

These are used by multiple tests - specifically those validating the device libraries that are linked in. This particular test is msan related, but the general device library linkage is also involved. I will keep as is. Thanks!

@mdtoguchi
Copy link
Contributor Author

@intel/llvm-gatekeepers, this is ready for merge.

@sarnex sarnex merged commit bbba4f9 into intel:sycl Jan 2, 2025
15 checks passed
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.

4 participants