[SYCL] Don't include level zero header explicitly to the interop header #2249
Conversation
Signed-off-by: Artur Gainullin <[email protected]>
| // This header should be included by users. | ||
| //#include <level_zero/ze_api.h> |
There was a problem hiding this comment.
Is there any "standard" macro defined in level_zero/ze_api.h, which we can check?
It would be better to have something like:
#if !defined(LEVEL_ZERO_INCLUDED)
#error "level_zero/ze_api.h must be included to use Level Zero interoperability with SYCL"
#endifinstead of these two lines.
There was a problem hiding this comment.
There is probably soemthing we could use, e.g. _ZE_API_H, but not sure how "standard" this is. Probably not (+@jandres742)
Also, since all the uses of L0 types are in templates, it is probably fine to have L0 included after this header
There was a problem hiding this comment.
I recalled that we tried the same approach with OpenCL headers in the past and it caused some issues, so we include OpenCL headers from SYCL headers.
What is the motivation for this change? Will it be clear for user that includes of Level Zero headers are missing and how to fix this problem?
There was a problem hiding this comment.
The ze_api.h has this
#ifndef _ZE_API_H
but not quite sure what we are trying to solve here. Why we cannot include the ze_api.h in the interop header? We know the interop header is gonna need it anyway, no?
There was a problem hiding this comment.
If I understand it correctly, this patch is supposed to address the compile time issue with missing headers on the systems w/o Intel GPU (e.g. cpu-only system or a system with NVIDIA GPU). In these cases DPC++ is not supposed to use Level Zero plug-in and there might be no headers on the system.
I'm not sure that asking users to provide headers in the best option here, but it's one way to address this issue.
There was a problem hiding this comment.
@bader You are right.
The following warning used to appear: SPIRVInstruction.h:3745:23: warning: ISO C++11 requires at least one argument for the "..." in a variadic macro _SPIRV_OP(ReadClockKHR) Original commit: KhronosGroup/SPIRV-LLVM-Translator@8155991
[DeviceASAN] Use device usm to sync asan runtime data instead of shared usm
Signed-off-by: Artur Gainullin [email protected]