Skip to content

Commit 9eaa344

Browse files
[SYCL] Assert for kernel having a name in runtime vs compile time (#20060)
Follow-up for #19990 Downstream CI uncovered another scenario where `detail::CompileTimeKernelInfo<KernelName>.Name` might be empty (other than non-SYCL compilation in some of the [unit]tests). That happens when compiling the same TU using multiple offload models (e.g., both SYCL and OMP) and the device compilation for non-SYCL model doesn't use SYCL integration header to provide kernel information. Ideally, we should be just guarding `static_assert`s with some `#if __SYCL_WHATEVER` but we don't set any SYCL-related macro when using 3rd party host compilers. Deal with that by turning compile-time `static_assert` into a run-time `assert` unless we can guarantee that SYCL kernel information is available. --------- Co-authored-by: Sergey Semenov <sergey.semenov@intel.com>
1 parent cb8b521 commit 9eaa344

File tree

3 files changed

+16
-19
lines changed

3 files changed

+16
-19
lines changed

sycl/include/sycl/handler.hpp

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -836,26 +836,28 @@ class __SYCL_EXPORT handler {
836836
Dims>());
837837
#endif
838838

839-
// SYCL unittests are built without sycl compiler, so "host" information
840-
// about kernels isn't provided (e.g., via integration headers or compiler
841-
// builtins).
842-
//
843-
// However, some copy/fill USM operation are implemented via SYCL kernels
844-
// and are instantiated resulting in all the `static_assert` checks being
845-
// exercised. Without kernel information that would fail, so we explicitly
846-
// disable such checks when this macro is defined. Note that the unittests
847-
// don't actually execute those operation, that's why disabling
848-
// unconditional `static_asserts`s is enough for now.
849-
#ifndef __SYCL_UNITTESTS_BYPASS_KERNEL_NAME_CHECK
850839
constexpr auto Info = detail::CompileTimeKernelInfo<KernelName>;
851840

852-
static_assert(Info.Name != std::string_view{}, "Kernel must have a name!");
841+
// Ideally, the following should be a `static_assert` but cannot do that as
842+
// it would fail in at least two scenarios:
843+
//
844+
// * Our own unittests that are compiled with an arbitrary host compiler
845+
// without SYCL knowledge (i.e., no integration headers
846+
// generated/supplied). Those that don't have to supply any stub kernel
847+
// information test exceptions thrown before kernel enqueue, so having a
848+
// run-time assert here doesn't affect them.
849+
//
850+
// * Mixed SYCL/OpenMP compilation, device code generation for OpenMP in
851+
// particular. For that scenario this code is compiled but never
852+
// executed.
853+
assert(Info.Name != std::string_view{} && "Kernel must have a name!");
853854

854855
// Some host compilers may have different captures from Clang. Currently
855856
// there is no stable way of handling this when extracting the captures,
856857
// so a static assert is made to fail for incompatible kernel lambdas.
857858
static_assert(
858-
sizeof(KernelType) == Info.KernelSize,
859+
Info.Name == std::string_view{} ||
860+
sizeof(KernelType) == Info.KernelSize,
859861
"Unexpected kernel lambda size. This can be caused by an "
860862
"external host compiler producing a lambda with an "
861863
"unexpected layout. This is a limitation of the compiler."
@@ -866,7 +868,6 @@ class __SYCL_EXPORT handler {
866868
"In case of MSVC, passing "
867869
"-fsycl-host-compiler-options='/std:c++latest' "
868870
"might also help.");
869-
#endif
870871

871872
setDeviceKernelInfo<KernelName>((void *)MHostKernel->getPtr());
872873

sycl/test/lit.cfg.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,7 @@
126126
llvm_symbolizer = os.path.join(config.llvm_build_bin_dir, "llvm-symbolizer")
127127
llvm_config.with_environment("LLVM_SYMBOLIZER_PATH", llvm_symbolizer)
128128

129-
sycl_host_only_options = (
130-
"-std=c++17 -Xclang -fsycl-is-host -D__SYCL_UNITTESTS_BYPASS_KERNEL_NAME_CHECK=1"
131-
)
129+
sycl_host_only_options = "-std=c++17 -Xclang -fsycl-is-host"
132130
for include_dir in [
133131
config.sycl_include,
134132
config.level_zero_include_dir,

sycl/unittests/CMakeLists.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ endforeach()
99

1010
add_compile_definitions(SYCL2020_DISABLE_DEPRECATION_WARNINGS SYCL_DISABLE_FSYCL_SYCLHPP_WARNING)
1111

12-
add_compile_definitions(__SYCL_UNITTESTS_BYPASS_KERNEL_NAME_CHECK)
13-
1412
# suppress warnings which came from Google Test sources
1513
if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
1614
add_compile_options("-Wno-suggest-override")

0 commit comments

Comments
 (0)