Repository navigation
[SYCL] fix e2e atomic ref tests - #23559
Conversation
|
@intel/llvm-gatekeepers |
| @@ -1,5 +1,8 @@ | |||
| // RUN: %if arch-intel_gpu_cri %{ %{build} -DFULL_ATOMIC16_COVERAGE -DFULL_ATOMIC32_COVERAGE -DFULL_ATOMIC64_COVERAGE -fsycl-device-code-split=per_kernel -Xspirv-translator -spirv-ext=+SPV_KHR_bfloat16,+SPV_INTEL_16bit_atomics -o %t.out %} | |||
| // RUN: %if arch-intel_gpu_cri %{ %{run} %t.out %} | |||
| // REQUIRES: arch-intel_gpu_cri | |||
There was a problem hiding this comment.
@dklochkov-emb Is there a particular reason we're limiting all atomic tests to CRI only?
There was a problem hiding this comment.
@againull yes, it is not confirmed that other drivers support atomics.
Moreover, all tests were disabled before.
There was a problem hiding this comment.
@againull in case of the build line
%if arch-intel_gpu_cri %{ %{build}..
fusion CI somehow skipped it but in case of run %if arch-intel_gpu_cri %{ %{run} %t.out %} somehow it tried to run resulting error that binary was not found
There was a problem hiding this comment.
@againull yes, it is not confirmed that other drivers support atomics.
It may apply to the new 16-bit atomics, but not to atomics generally: 32-bit atomics are required by SYCL, and the existing tests were already running and passing on other platforms, including PVC.(https://github.com/intel/llvm/actions/runs/35870937631/job/107222525795).
For the 16-bit tests, we should at least use REQUIRES: aspect-ext_oneapi_atomic16 instead of hardcoding arch-intel_gpu_cri. Ideally, the 16-bit coverage should be in separate test wrappers: combining it with the existing 32-bit tests under a file-wide requirement causes us to lose 32-bit coverage on devices without atomic16 support.
Moreover, all tests were disabled before.
Could you elaborate on this? In this older PVC run (https://github.com/intel/llvm/actions/runs/35870937631/job/107222525795), the atomic tests were executed and passed. Were you referring specifically to the 16-bit cases?
4 participants
This PR fixes issues of atomic_ref e2e tests. When Fusion CI runs it, build and run lines have different "calculations" of conditions. This makes possible situation when if some_arch will be true for RUN of
%{run}and false for run of%{build}.The reason why I left this in original PR, I did not want to remove unconditional build and run, However, they are useless: they does not build and run any test.