-
Notifications
You must be signed in to change notification settings - Fork 808
[SYCL[E2E] Enable Basic/image tests #20959
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
b9d047a to
19727b3
Compare
19727b3 to
072dfa0
Compare
072dfa0 to
a8efae5
Compare
|
@intel/llvm-reviewers-runtime This is kind invitation to review |
uditagarwal97
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. Please update the corresponding Github issues, either close them or update their description, if the PR doesn't fully address the issue.
|
@intel/llvm-gatekeepers please consider merging |
|
|
||
| // Test flakily fails on PVC and BMG. | ||
| // UNSUPPORTED: arch-intel_gpu_pvc || arch-intel_gpu_bmg_g21 | ||
| // UNSUPPORTED-TRACKER: https://github.com/intel/llvm/issues/16401 |
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.
Could you please clarify, does "We no longer support opencl:gpu" mean "we won't fix it"?
In this case, I'd say it might be better to mark this test as:
// UNSUPPORTED: opencl && (arch-intel_gpu_pvc || arch-intel_gpu_bmg_g21)
// UNSUPPORTED-INTENDED: *justification*
Just in case it's launched locally / internally.
UPD. BTW we still test PVC & opencl:gpu in pre-commit, so if it wasn't fixed, it will appear again.
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.
It was my mistake, I will verify the accessor test again before merge and re-consider the modification. Thanks
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.
@KornevNikita I reproduced the bug on opencl:gpu as described in tracker, so fixed the test. Thanks again for being watchful.
|
@intel/llvm-gatekeepers This is ready for merge |
These tests no longer fail.
The accessor.cpp test was reported to fail only with opencl:gpu device
The image/ tests pass locally and in CI