From 218a5fb6dda9c639750103f51aaa0d1f25010d0c Mon Sep 17 00:00:00 2001 From: Jiaming Yuan Date: Tue, 9 Feb 2021 08:12:58 +0800 Subject: [PATCH] Simplify Span checks. (#6685) * Stop printing out message. * Remove R specialization. The printed message is not really useful anyway, without a reproducible example there's no way to fix it. But if there's a reproducible example, we can always obtain these information by a debugger. Removing the `printf` function avoids creating the context in kernel. --- include/xgboost/span.h | 61 +++++++++++++++++++---------------- tests/cpp/common/test_span.cc | 52 ++++++++++++++--------------- 2 files changed, 59 insertions(+), 54 deletions(-) diff --git a/include/xgboost/span.h b/include/xgboost/span.h index 2492a62a1..665650d7f 100644 --- a/include/xgboost/span.h +++ b/include/xgboost/span.h @@ -38,6 +38,10 @@ #include #include +#if defined(__CUDACC__) +#include +#endif // defined(__CUDACC__) + /*! * The version number 1910 is picked up from GSL. * @@ -71,45 +75,46 @@ namespace xgboost { namespace common { +#if defined(__CUDA_ARCH__) // Usual logging facility is not available inside device code. -// assert is not supported in mac as of CUDA 10.0 + +#if defined(_MSC_VER) + +// Windows CUDA doesn't have __assert_fail. #define KERNEL_CHECK(cond) \ do { \ - if (!(cond)) { \ - printf("\nKernel error:\n" \ - "In: %s: %d\n" \ - "\t%s\n\tExpecting: %s\n" \ - "\tBlock: [%d, %d, %d], Thread: [%d, %d, %d]\n\n", \ - __FILE__, __LINE__, __PRETTY_FUNCTION__, #cond, blockIdx.x, \ - blockIdx.y, blockIdx.z, threadIdx.x, threadIdx.y, threadIdx.z); \ + if (XGBOOST_EXPECT(!(cond), false)) { \ asm("trap;"); \ } \ - } while (0); + } while (0) + +#else // defined(_MSC_VER) + +#define __ASSERT_STR_HELPER(x) #x + +#define KERNEL_CHECK(cond) \ + (XGBOOST_EXPECT((cond), true) \ + ? static_cast(0) \ + : __assert_fail(__ASSERT_STR_HELPER(e), __FILE__, __LINE__, \ + __PRETTY_FUNCTION__)) + +#endif // defined(_MSC_VER) -#if defined(__CUDA_ARCH__) #define SPAN_CHECK KERNEL_CHECK -#elif defined(XGBOOST_STRICT_R_MODE) && XGBOOST_STRICT_R_MODE == 1 // R package -#define SPAN_CHECK CHECK // check from dmlc -#else // not CUDA, not R -#define SPAN_CHECK(cond) \ - do { \ - if (XGBOOST_EXPECT(!(cond), false)) { \ - fprintf(stderr, "[xgboost] Condition %s failed.\n", #cond); \ - fflush(stderr); /* It seems stderr on Windows is beffered? */ \ - std::terminate(); \ - } \ - } while (0); + +#else // not CUDA + +#define KERNEL_CHECK(cond) \ + (XGBOOST_EXPECT((cond), true) ? static_cast(0) : std::terminate()) + +#define SPAN_CHECK(cond) KERNEL_CHECK(cond) + #endif // __CUDA_ARCH__ #if defined(__CUDA_ARCH__) -#define SPAN_LT(lhs, rhs) \ - if (!((lhs) < (rhs))) { \ - printf("[xgboost] Condition: %lu < %lu failed\n", \ - static_cast(lhs), static_cast(rhs)); \ - asm("trap;"); \ - } +#define SPAN_LT(lhs, rhs) KERNEL_CHECK((lhs) < (rhs)) #else -#define SPAN_LT(lhs, rhs) SPAN_CHECK((lhs) < (rhs)) +#define SPAN_LT(lhs, rhs) KERNEL_CHECK((lhs) < (rhs)) #endif // defined(__CUDA_ARCH__) namespace detail { diff --git a/tests/cpp/common/test_span.cc b/tests/cpp/common/test_span.cc index b743b2d6e..3ee99c0ae 100644 --- a/tests/cpp/common/test_span.cc +++ b/tests/cpp/common/test_span.cc @@ -122,7 +122,7 @@ TEST(SpanDeathTest, FromPtrLen) { InitializeRange(arr, arr+16); { auto lazy = [=]() {Span tmp (arr, 5);}; - EXPECT_DEATH(lazy(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(lazy(), ""); } } @@ -296,11 +296,11 @@ TEST(SpanDeathTest, ElementAccess) { InitializeRange(arr, arr + 16); Span s (arr); - EXPECT_DEATH(s[16], "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s[-1], "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s[16], ""); + EXPECT_DEATH(s[-1], ""); - EXPECT_DEATH(s(16), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s(-1), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s(16), ""); + EXPECT_DEATH(s(-1), ""); } TEST(Span, Obversers) { @@ -327,13 +327,13 @@ TEST(Span, FrontBack) { TEST(SpanDeathTest, FrontBack) { { Span s; - EXPECT_DEATH(s.front(), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s.back(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.front(), ""); + EXPECT_DEATH(s.back(), ""); } { Span s; - EXPECT_DEATH(s.front(), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s.back(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.front(), ""); + EXPECT_DEATH(s.back(), ""); } } @@ -411,9 +411,9 @@ TEST(SpanDeathTest, FirstLast) { Span s (arr); auto constexpr kOne = static_cast::index_type>(-1); - EXPECT_DEATH(s.first(), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s.first<17>(), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s.first<32>(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.first(), ""); + EXPECT_DEATH(s.first<17>(), ""); + EXPECT_DEATH(s.first<32>(), ""); } { @@ -422,9 +422,9 @@ TEST(SpanDeathTest, FirstLast) { Span s (arr); auto constexpr kOne = static_cast::index_type>(-1); - EXPECT_DEATH(s.last(), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s.last<17>(), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s.last<32>(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.last(), ""); + EXPECT_DEATH(s.last<17>(), ""); + EXPECT_DEATH(s.last<32>(), ""); } // dynamic extent @@ -432,9 +432,9 @@ TEST(SpanDeathTest, FirstLast) { float *arr = new float[16]; InitializeRange(arr, arr + 16); Span s (arr, 16); - EXPECT_DEATH(s.first(-1), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s.first(17), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s.first(32), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.first(-1), ""); + EXPECT_DEATH(s.first(17), ""); + EXPECT_DEATH(s.first(32), ""); delete [] arr; } @@ -443,9 +443,9 @@ TEST(SpanDeathTest, FirstLast) { float *arr = new float[16]; InitializeRange(arr, arr + 16); Span s (arr, 16); - EXPECT_DEATH(s.last(-1), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s.last(17), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s.last(32), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.last(-1), ""); + EXPECT_DEATH(s.last(17), ""); + EXPECT_DEATH(s.last(32), ""); delete [] arr; } @@ -469,12 +469,12 @@ TEST(Span, Subspan) { TEST(SpanDeathTest, Subspan) { int arr[16] {0}; Span s1 (arr); - EXPECT_DEATH(s1.subspan(-1, 0), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s1.subspan(17, 0), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s1.subspan(-1, 0), ""); + EXPECT_DEATH(s1.subspan(17, 0), ""); auto constexpr kOne = static_cast::index_type>(-1); - EXPECT_DEATH(s1.subspan(), "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_DEATH(s1.subspan<17>(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s1.subspan(), ""); + EXPECT_DEATH(s1.subspan<17>(), ""); } TEST(Span, Compare) { @@ -523,7 +523,7 @@ TEST(SpanDeathTest, Empty) { std::vector data(1, 0); ASSERT_TRUE(data.data()); Span s{data.data(), Span::index_type(0)}; // ok to define 0 size span. - EXPECT_DEATH(s[0], "\\[xgboost\\] Condition .* failed.\n"); // not ok to use it. + EXPECT_DEATH(s[0], ""); // not ok to use it. } } // namespace common