[SYCL] Rely on automatic enabling of native/emulated FP atomics #308
Conversation
After OpenCL CPU support of SPV_EXT_shader_atomic_float_* has arrived, the compiler is switched to defining the SYCL_USE_NATIVE_FP_ATOMICS internally for non-FPGA, non-NVPTX targets. Any further compiler changes that may come after NVPTX/FPGA HW support lands will not require test change, as the guarantee will remain that correct (native or emulated) versions of FP atomic functions get emitted and executed by the HW. Signed-off-by: Artem Gindinson <[email protected]>
Signed-off-by: Artem Gindinson <[email protected]>
AGindinson
force-pushed
the
native-fp-atomics
branch
from
ba2eda4 to
96d031c
Compare
AGindinson
changed the title
|
This tests intel/llvm#3869. This change to the test suite needs to be merged first so that the compiler PR can be verified based off that; the effect will be that until the compiler PR is merged, native atomic functionality will not be tested. |
There was a problem hiding this comment.
- Why do you remove tests testing emulated workflow? I think it's still useful for the target that do not support FP atomics natively.
- How do you know that tests pass because FP atomics are supported natively and not emulated? I don't see any checks for that. The test just verifies that FP atomics are functional.
| // RUN: %HOST_RUN_PLACEHOLDER %t.out | ||
| // RUN: %CPU_RUN_PLACEHOLDER %t.out | ||
| // RUN: %GPU_RUN_PLACEHOLDER %t.out | ||
| // RUN: %CPU_RUN_PLACEHOLDER %t.out |
There was a problem hiding this comment.
This is the issue with how Git concatenates the two commits of this PR (the actual changes w/ deleted/moved files). It may be easier to review the actual changes by looking at f66a122 specifically - the reordering would not be shown there.
Once the tested compiler PR is merged, this is exactly what will be happening - CUDA and FPGA will be tested with emulated FP atomic functions, while SPIR/CPU/Intel GPU compilations will receive the
The driver PR that is being tested by these changes will ensure that; the driver unit tests verify that the macro is passed for the targets that support the functionality. That said, please note my earlier comment: the test coverage will, in fact, be temporarily reduced to "just functional verficiation" until the compiler changes come into effect. |
|
@bader, could you please revisit the PR and merge if the approach looks good? |
…l#308) After OpenCL CPU support of SPV_EXT_shader_atomic_float_* has arrived, the compiler is switched to defining the SYCL_USE_NATIVE_FP_ATOMICS internally for non-FPGA, non-NVPTX targets. Any further compiler changes that may come after NVPTX/FPGA HW support lands will not require test change, as the guarantee will remain that correct (native or emulated) versions of FP atomic functions get emitted and executed by the HW. Signed-off-by: Artem Gindinson <[email protected]>
…l/llvm-test-suite#308) After OpenCL CPU support of SPV_EXT_shader_atomic_float_* has arrived, the compiler is switched to defining the SYCL_USE_NATIVE_FP_ATOMICS internally for non-FPGA, non-NVPTX targets. Any further compiler changes that may come after NVPTX/FPGA HW support lands will not require test change, as the guarantee will remain that correct (native or emulated) versions of FP atomic functions get emitted and executed by the HW. Signed-off-by: Artem Gindinson <[email protected]>
After OpenCL CPU support of SPV_EXT_shader_atomic_float_* has arrived,
the compiler is switched to defining the SYCL_USE_NATIVE_FP_ATOMICS
internally for non-FPGA, non-NVPTX targets. Any further compiler changes
that may come after NVPTX/FPGA HW support lands will not require test
change, as the guarantee will remain that correct (native or emulated)
versions of FP atomic functions get emitted and executed by the HW.
Signed-off-by: Artem Gindinson [email protected]