[SYCL] Prevent fallback assert postprocessor being passed to CUDA and HIP devices

[AidanBeltonS]

This patch fixes an error with HIP backend where tests DeviceCodeSplit/split-per-kernel.cpp and DeviceCodeSplit/split-per-source-main.cpp fail with Memory access fault by GPU node-4 (Agent handle: 0x709c50) on address (nil). Reason: Page not present or supervisor privilege.

This is due to a postprocessor being sent to queue_impl::submit_impl.
The patch add checks to queue.hpp so postprocessors needed for fallback assert do not get passed for HIP and CUDA devices. Neither currently support fallback asserts.

Note: adding __AMDGCN__ to

#if !defined(SYCL_DISABLE_FALLBACK_ASSERT) && !defined(__NVPTX__)
#define __SYCL_USE_FALLBACK_ASSERT 1
#else
#define __SYCL_USE_FALLBACK_ASSERT 0
#endif

was not a fix as a new error arose in its place. Investigation showed that macros __AMDGCN__ and __NVPTX__ are only set for device compilation and as a result did not stop preprocessors being passed to submit_impl. I think the postprocesor was not supposed to be passed to CUDA devices. Despite being passed it was not causing an error for CUDA just HIP.

[romanovvlad]

This patch fixes an error with HIP backend where tests DeviceCodeSplit/split-per-kernel.cpp and DeviceCodeSplit/split-per-source-main.cpp fail with Memory access fault by GPU node-4 (Agent handle: 0x709c50) on address (nil). Reason: Page not present or supervisor privilege.

This is due to a postprocessor being sent to queue_impl::submit_impl.

Could you please clarify if this patch is a workaround until the issue is root caused and a proper solution found or a final solution? Do we know what the root cause is?

[AidanBeltonS]

Could you please clarify if this patch is a workaround until the issue is root caused and a proper solution found or a final solution? Do we know what the root cause is?

The root cause is that target macro's like __NVPTX__ and __AMDGCN__ are being used to prevent sending postprocessors to certain devices. These macros are not specified when building for the host so it does not prevent sending a postprocessor.
This causes errors on HIP with module splitting.

This is proposed as a long-term solution until CUDA and HIP support fallback asserts.

[s-kanaev]

I think the postprocesor was not supposed to be passed to CUDA devices.

Even though it was passed for CUDA device, it converted was sort of a NOP due CUDA backend reported support for native assert. See the change in #3767

[AidanBeltonS]

Even though it was passed for CUDA device, it converted was sort of a NOP due CUDA backend reported support for native assert. See the change in #3767

Ahh, thanks for explaining. I was wondering why CUDA devices were not failing.

[s-kanaev]

Even though it was passed for CUDA device, it converted was sort of a NOP due CUDA backend reported support for native assert. See the change in #3767

Ahh, thanks for explaining. I was wondering why CUDA devices were not failing.

@AidanBeltonS , could you, then, modify the HIP plugin to report assert being supported? This will remain {{queue.hpp}} back-end agnostic.

[AidanBeltonS]

@AidanBeltonS , could you, then, modify the HIP plugin to report assert being supported? This will remain {{queue.hpp}} back-end agnostic.

I have tested adding PI_DEVICE_INFO_EXTENSION_DEVICELIB_ASSERT to the hip extensions. This resolves the problem.

Currently HIP4.3 does not support asserts natively. Though it seems that it will soon. Are okay with adding the extension regardless of HIPs support? I can comment that this is being used resolve this issue and add a note to remove the comment once native asserts are supported.

From HIP 4.3 docs

The assert function is under development. HIP does support an "abort" call which will terminate the process execution from inside the kernel.

[bader]

@smaslov-intel, @romanovvlad, @s-kanaev, ping.

[bader]

@s-kanaev, ping.

[s-kanaev]

Seems legit