[SYCL][HIP] Add assert support #6424
Conversation
This patch adds an implementation of `__assert_fail` for HIP. It is implemented in IR because the function needs to take pointers with no address space and OpenCL compilation makes them private. It is implemented using the OCKL printf functions provided by HIP. In addition this patch also fixes `SYCL_EXTERNAL` for HIP, the AMDGPU backend runs some dead code elimination and would simply delete `SYCL_EXTERNAL` functions, and so this patch prevents the AMDGPU backend from deleting `SYCL_EXTERNAL` functions, identifying them through the `sycl-module-id` attribute (as done in other places such as `sycl-post-link`). This issue showed up in some of the assert tests that use multiple translation units.
|
/verify with intel/llvm-test-suite#1083 |
|
Did it succeed the run with the The other entries look as expected, except for the CUDA failure, has that test been flaky? I don't really see how this patch could cause any failures for the CUDA plugin. |
intel/llvm-test-suite#1083 enables execution of some tests on HIP platform. AFAIK, Jenkins covers only OpenCL and Level Zero "platforms", so running this testing does not give any additional information unfortunately. |
|
Okay, I've re-run this a lot locally with intel/llvm-test-suite#1083 and I'm not seeing any flakiness in the assert tests, it's slightly different than the CUDA asserts because the HIP driver will Given that and since we can't get more information from the Jenkins testing for HIP at the moment, would it be okay to merge the PRs as-is and we can revisit if the tests turn out to be flaky on the CI? |
|
@npmiller, I'm merging it assuming that your local testing is clean. |
This patch adds an implementation of
__assert_failfor HIP. It isimplemented in IR because the function needs to take pointers with no
address space and OpenCL compilation makes them private. It is
implemented using the OCKL printf functions provided by HIP.
In addition this patch also fixes
SYCL_EXTERNALfor HIP, the AMDGPUbackend runs some dead code elimination and would simply delete
SYCL_EXTERNALfunctions, and so this patch prevents the AMDGPU backendfrom deleting
SYCL_EXTERNALfunctions, identifying them throughthe
sycl-module-idattribute (as done in other places such assycl-post-link). This issue showed up in some of the assert tests thatuse multiple translation units, and this patch also fixes the
sycl-external.cpptest.Matching
llvm-test-suiteupdate: intel/llvm-test-suite#1083