[SYCL] Fix device assertion bug on windows (#15232)

This PR fixes a device assertion bug on Windows where the assertion message and information are not printed when an assertion is triggered. Also re-enabled some tests failing because of it. See #12797.
intel · Sep 2, 2024 · 101307f · 101307f
1 parent 56a6ae2
commit 101307f
Show file tree

Hide file tree

Showing 12 changed files with 100 additions and 20 deletions.
diff --git a/sycl/include/sycl/stl_wrappers/assert.h b/sycl/include/sycl/stl_wrappers/assert.h
@@ -0,0 +1,44 @@
+//== ----------------<assert.h> wrapper around STL--------------------------==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Must not be guarded. C++ standard says the macro assert is redefined
+// according to the current state of NDEBUG each time that <cassert> is
+// included.
+
+#if defined(__has_include_next)
+#include_next <assert.h>
+#else
+#include <../ucrt/assert.h>
+#endif
+
+#ifdef __SYCL_DEVICE_ONLY__
+#include <CL/__spirv/spirv_vars.hpp>
+
+// Device assertions on Windows do not work properly so we define these wrappers
+// around the STL assertion headers cassert and assert.h where we redefine
+// the assert macro to call __devicelib_assert_fail directly and bypass
+// _wassert.
+#if defined(_WIN32) && defined(assert)
+extern "C" __DPCPP_SYCL_EXTERNAL void
+__devicelib_assert_fail(const char *, const char *, int32_t, const char *,
+                        uint64_t, uint64_t, uint64_t, uint64_t, uint64_t,
+                        uint64_t);
+#undef assert
+#if defined(NDEBUG)
+#define assert(e) ((void)0)
+#else
+#define assert(e)                                                              \
+  ((e) ? void(0)                                                               \
+       : __devicelib_assert_fail(                                              \
+             #e, __FILE__, __LINE__, nullptr, __spirv_GlobalInvocationId_x(),  \
+             __spirv_GlobalInvocationId_y(), __spirv_GlobalInvocationId_z(),   \
+             __spirv_LocalInvocationId_x(), __spirv_LocalInvocationId_y(),     \
+             __spirv_LocalInvocationId_z()))
+#endif
+#endif
+#endif
diff --git a/sycl/include/sycl/stl_wrappers/cassert b/sycl/include/sycl/stl_wrappers/cassert
@@ -0,0 +1,44 @@
+//==---------------- <cassert> wrapper around STL --------------------------==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Must not be guarded. C++ standard says the macro assert is redefined
+// according to the current state of NDEBUG each time that <cassert> is
+// included.
+
+#if defined(__has_include_next)
+#include_next <cassert>
+#else
+#include <../include/cassert>
+#endif
+
+#ifdef __SYCL_DEVICE_ONLY__
+#include <CL/__spirv/spirv_vars.hpp>
+
+// Device assertions on Windows do not work properly so we define these wrappers
+// around the STL assertion headers cassert and assert.h where we redefine
+// the assert macro to call __devicelib_assert_fail directly and bypass
+// _wassert.
+#if defined(_WIN32) && defined(assert)
+extern "C" __DPCPP_SYCL_EXTERNAL void
+__devicelib_assert_fail(const char *, const char *, int32_t, const char *,
+                        uint64_t, uint64_t, uint64_t, uint64_t, uint64_t,
+                        uint64_t);
+#undef assert
+#if defined(NDEBUG)
+#define assert(e) ((void)0)
+#else
+#define assert(e)                                                              \
+  ((e) ? void(0)                                                               \
+       : __devicelib_assert_fail(                                              \
+             #e, __FILE__, __LINE__, nullptr, __spirv_GlobalInvocationId_x(),  \
+             __spirv_GlobalInvocationId_y(), __spirv_GlobalInvocationId_z(),   \
+             __spirv_LocalInvocationId_x(), __spirv_LocalInvocationId_y(),     \
+             __spirv_LocalInvocationId_z()))
+#endif
+#endif
+#endif
diff --git a/sycl/test-e2e/Assert/assert_in_kernels_win.cpp b/sycl/test-e2e/Assert/assert_in_kernels_win.cpp
@@ -1,9 +1,7 @@
-// https://github.com/intel/llvm/issues/12797
-// UNSUPPORTED: windows
 // REQUIRES: windows
 // RUN: %{build} -DSYCL_FALLBACK_ASSERT=1 -o %t.out
 // Shouldn't fail on ACC as fallback assert isn't enqueued there
-// RUN: %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if acc %{ --check-prefix=CHECK-ACC %}
+// RUN: %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if fpga %{ --check-prefix=CHECK-ACC %}
 //
 // CHECK-NOT:  One shouldn't see this message
 // FIXME Windows version prints '(null)' instead of '<unknown func>' once in a

diff --git a/sycl/test-e2e/Assert/assert_in_multiple_tus_one_ndebug_win.cpp b/sycl/test-e2e/Assert/assert_in_multiple_tus_one_ndebug_win.cpp
@@ -1,9 +1,7 @@
-// https://github.com/intel/llvm/issues/12797
-// UNSUPPORTED: windows
 // REQUIRES: windows
 // RUN: %clangxx -DSYCL_FALLBACK_ASSERT=1 -fsycl -fsycl-targets=%{sycl_triple} -DDEFINE_NDEBUG_INFILE2 -I %S/Inputs %S/assert_in_multiple_tus.cpp %S/Inputs/kernels_in_file2.cpp -o %t.out
 // Shouldn't fail on ACC as fallback assert isn't enqueued there
-// RUN: %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if acc %{ --check-prefix=CHECK-ACC %}
+// RUN: %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if fpga %{ --check-prefix=CHECK-ACC %}
 //
 // CHECK-NOT:  this message from calculus
 // FIXME Windows version prints '(null)' instead of '<unknown func>' once in a

diff --git a/sycl/test-e2e/Assert/assert_in_multiple_tus_win.cpp b/sycl/test-e2e/Assert/assert_in_multiple_tus_win.cpp
@@ -1,9 +1,7 @@
-// https://github.com/intel/llvm/issues/12797
-// UNSUPPORTED: windows
 // REQUIRES: windows
 // RUN: %{build} -DSYCL_FALLBACK_ASSERT=1 -I %S/Inputs %S/Inputs/kernels_in_file2.cpp -o %t.out
 // Shouldn't fail on ACC as fallback assert isn't enqueued there
-// RUN: %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if acc %{ --check-prefix=CHECK-ACC %}
+// RUN: %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if fpga %{ --check-prefix=CHECK-ACC %}
 //
 // FIXME Windows version prints '(null)' instead of '<unknown func>' once in a
 // while for some insane reason.

diff --git a/sycl/test-e2e/Assert/assert_in_one_kernel_win.cpp b/sycl/test-e2e/Assert/assert_in_one_kernel_win.cpp
@@ -1,9 +1,7 @@
-// https://github.com/intel/llvm/issues/12797
-// UNSUPPORTED: windows
 // REQUIRES: windows
 // RUN: %{build} -DSYCL_FALLBACK_ASSERT=1 -o %t.out
 // Shouldn't fail on ACC as fallback assert isn't enqueued there
-// RUN: %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if acc %{ --check-prefix=CHECK-ACC %}
+// RUN: %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if fpga %{ --check-prefix=CHECK-ACC %}
 //
 // FIXME Windows version prints '(null)' instead of '<unknown func>' once in a
 // while for some insane reason.

diff --git a/sycl/test-e2e/Assert/assert_in_simultaneous_kernels_win.cpp b/sycl/test-e2e/Assert/assert_in_simultaneous_kernels_win.cpp
@@ -1,5 +1,3 @@
-// https://github.com/intel/llvm/issues/12797
-// UNSUPPORTED: windows
 // REQUIRES: windows
 // RUN: %{build} -DSYCL_FALLBACK_ASSERT=1 -o %t.out %threads_lib
 //
@@ -12,7 +10,7 @@
 // DEFINE: %{gpu_env} = env SYCL_PI_LEVEL_ZERO_TRACK_INDIRECT_ACCESS_MEMORY=1 SYCL_PI_SUPPRESS_ERROR_MESSAGE=1
 
 // Shouldn't fail on ACC as fallback assert isn't enqueued there
-// RUN: %if gpu %{ %{gpu_env} %} %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if acc %{ --check-prefix=CHECK-ACC %}
+// RUN: %if gpu %{ %{gpu_env} %} %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if fpga %{ --check-prefix=CHECK-ACC %}
 //
 // FIXME Windows version prints '(null)' instead of '<unknown func>' once in a
 // while for some insane reason.

diff --git a/sycl/test-e2e/Assert/assert_in_simultaneously_multiple_tus.cpp b/sycl/test-e2e/Assert/assert_in_simultaneously_multiple_tus.cpp
@@ -4,8 +4,6 @@
 // FIXME: Remove XFAIL one intel/llvm#11364 is resolved
 // XFAIL: (opencl && gpu)
 //
-// https://github.com/intel/llvm/issues/12797
-// UNSUPPORTED: windows
 //
 // RUN: %{build} -DSYCL_FALLBACK_ASSERT=1 -I %S/Inputs %S/Inputs/kernels_in_file2.cpp -o %t.out %threads_lib
 //

diff --git a/sycl/test-e2e/Assert/assert_in_simultaneously_multiple_tus_one_ndebug.cpp b/sycl/test-e2e/Assert/assert_in_simultaneously_multiple_tus_one_ndebug.cpp
@@ -5,8 +5,6 @@
 // FIXME: Remove XFAIL one intel/llvm#11364 is resolved
 // XFAIL: (opencl && gpu)
 //
-// https://github.com/intel/llvm/issues/12797
-// UNSUPPORTED: windows
 //
 // RUN: %clangxx -DSYCL_FALLBACK_ASSERT=1 -fsycl -fsycl-targets=%{sycl_triple} -DDEFINE_NDEBUG_INFILE2 -I %S/Inputs %S/assert_in_simultaneously_multiple_tus.cpp %S/Inputs/kernels_in_file2.cpp -o %t.out %threads_lib
 // RUN: %if cpu %{ %{run} %t.out &> %t.cpu.txt ; FileCheck %s --input-file %t.cpu.txt %}

diff --git a/sycl/test/include_deps/sycl_accessor.hpp.cpp b/sycl/test/include_deps/sycl_accessor.hpp.cpp
@@ -53,6 +53,8 @@
 // CHECK-NEXT: detail/boost/mp11/detail/mp_remove_if.hpp
 // CHECK-NEXT: detail/boost/mp11/detail/mp_map_find.hpp
 // CHECK-NEXT: detail/boost/mp11/detail/mp_with_index.hpp
+// CHECK-NEXT: stl_wrappers/cassert
+// CHECK-NEXT: stl_wrappers/assert.h
 // CHECK-NEXT: detail/boost/mp11/integer_sequence.hpp
 // CHECK-NEXT: buffer.hpp
 // CHECK-NEXT: backend_types.hpp

diff --git a/sycl/test/include_deps/sycl_buffer.hpp.cpp b/sycl/test/include_deps/sycl_buffer.hpp.cpp
@@ -22,9 +22,11 @@
 // CHECK-NEXT: detail/string.hpp
 // CHECK-NEXT: ur_api.h
 // CHECK-NEXT: detail/common.hpp
+// CHECK-NEXT: stl_wrappers/cassert
+// CHECK-NEXT: stl_wrappers/assert.h
+// CHECK-NEXT: CL/__spirv/spirv_vars.hpp
 // CHECK-NEXT: detail/helpers.hpp
 // CHECK-NEXT: memory_enums.hpp
-// CHECK-NEXT: CL/__spirv/spirv_vars.hpp
 // CHECK-NEXT: detail/iostream_proxy.hpp
 // CHECK-NEXT: detail/is_device_copyable.hpp
 // CHECK-NEXT: detail/owner_less_base.hpp

diff --git a/sycl/test/include_deps/sycl_detail_core.hpp.cpp b/sycl/test/include_deps/sycl_detail_core.hpp.cpp
@@ -54,6 +54,8 @@
 // CHECK-NEXT: detail/boost/mp11/detail/mp_remove_if.hpp
 // CHECK-NEXT: detail/boost/mp11/detail/mp_map_find.hpp
 // CHECK-NEXT: detail/boost/mp11/detail/mp_with_index.hpp
+// CHECK-NEXT: stl_wrappers/cassert
+// CHECK-NEXT: stl_wrappers/assert.h
 // CHECK-NEXT: detail/boost/mp11/integer_sequence.hpp
 // CHECK-NEXT: buffer.hpp
 // CHECK-NEXT: backend_types.hpp