From 0dfeee1108fa22e9fbc22a48d4dd8de9285a9818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20Sch=C3=BCtt?= Date: Wed, 13 Dec 2023 21:49:22 +0100 Subject: [PATCH] Drop support for NDEBUG --- CMakeLists.txt | 10 ++++++++-- INSTALL.md | 4 ---- src/CMakeLists.txt | 12 ++++-------- src/base/base_uses.f90 | 5 ----- src/cp2k_info.F | 3 --- src/environment.F | 7 ++++++- src/grid/common/grid_library.c | 5 +++++ src/grid/hip/grid_hip_context.cu | 2 -- tools/docker/scripts/test_regtest_cmake.sh | 2 +- tools/precommit/check_file_properties.py | 1 + 10 files changed, 25 insertions(+), 26 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fb03bafd7c..c86eaa6d1d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,6 +58,12 @@ endif() set(CMAKE_CXX_STANDARD 14) set(CMAKE_C_STANDARD 11) +# remove NDEBUG flag +string(REPLACE "-DNDEBUG" "" CMAKE_C_FLAGS_RELEASE ${CMAKE_C_FLAGS_RELEASE}) +string(REPLACE "-DNDEBUG" "" CMAKE_CXX_FLAGS_RELEASE ${CMAKE_CXX_FLAGS_RELEASE}) +string(REPLACE "-DNDEBUG" "" CMAKE_Fortran_FLAGS_RELEASE + ${CMAKE_Fortran_FLAGS_RELEASE}) + find_package(PkgConfig) # ############################################################################## @@ -549,11 +555,11 @@ elseif(CP2K_USE_ACCEL MATCHES "HIP") set(CMAKE_HIP_ARCHITECTURES gfx801 gfx900 gfx90a) if(NOT CMAKE_BUILD_TYPE) - set(HIP_RELEASE_OPTIONS "-O3 -DNDEBUG --std=c++11") + set(HIP_RELEASE_OPTIONS "-O3 --std=c++11") elseif(${CMAKE_BUILD_TYPE} STREQUAL "RelWithDebInfo") set(HIP_RELEASE_OPTIONS "-O2 -g --std=c++11") elseif(${CMAKE_BUILD_TYPE} STREQUAL "Release") - set(HIP_RELEASE_OPTIONS "-O3 -DNDEBUG --std=c++11") + set(HIP_RELEASE_OPTIONS "-O3 --std=c++11") elseif(${CMAKE_BUILD_TYPE} STREQUAL "Debug") set(HIP_RELEASE_OPTIONS "-O0 -g --std=c++11") endif() diff --git a/INSTALL.md b/INSTALL.md index 7ffc3408c0..89620d1b00 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -454,10 +454,6 @@ architectures can be found in [arch folder](./arch/). The names of these files m Alternatively, provides sample arch files as part of the testing reports (click on the status field, search for 'ARCH-file'). -- With -DNDEBUG assertions may be stripped ("compiled out"). -- NDEBUG is the ANSI-conforming symbol name (not \_\_NDEBUG). -- Regular release builds may carry assertions for safety. - Conventionally, there are six versions: | Acronym | Meaning | diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a519705e9a..23dcf97eb3 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1578,7 +1578,6 @@ target_compile_definitions( __COMPILE_REVISION=\"${CP2K_GIT_HASH}\" __DATA_DIR=\"${CMAKE_INSTALL_FULL_DATAROOTDIR}/cp2k/data\" __COMPILE_ARCH=\"${CMAKE_SYSTEM_PROCESSOR}\" - $<$:NDEBUG> $<$:__NO_ABORT> $<$:__HAS_IEEE_EXCEPTIONS> $<$:__CHECK_DIAG> @@ -1690,9 +1689,8 @@ endforeach() add_executable(dbm_miniapp dbm/dbm_miniapp.c ${CP2K_DBM_SRCS_C} ${CP2K_DBM_SRCS_GPU} ${CP2K_OFFLOAD_SRCS_C}) target_compile_definitions( - dbm_miniapp - PRIVATE $<$:NDEBUG> $<$:__LIBXSMM> - $<$:__MKL>) + dbm_miniapp PRIVATE $<$:__LIBXSMM> + $<$:__MKL>) if(CP2K_DISABLE_DBM_GPU) target_compile_definitions(dbm_miniapp PRIVATE __NO_OFFLOAD_DBM) @@ -1709,10 +1707,8 @@ foreach(__app grid_miniapp grid_unittest) $<$:${CP2K_GRID_SRCS_HIP}> ${CP2K_OFFLOAD_SRCS_C}) target_compile_definitions( - ${__app} - PRIVATE $<$:NDEBUG> - $<$:__LIBXSMM> - $<$:__MKL>) + ${__app} PRIVATE $<$:__LIBXSMM> + $<$:__MKL>) if(CP2K_DISABLE_GRID_GPU) target_compile_definitions(${__app} PUBLIC __NO_OFFLOAD_GRID) diff --git a/src/base/base_uses.f90 b/src/base/base_uses.f90 index 4459734eb8..106072a187 100644 --- a/src/base/base_uses.f90 +++ b/src/base/base_uses.f90 @@ -32,12 +32,7 @@ ! In contrast to CPWARN, the warning counter is not increased #define CPHINT(msg) CALL cp__h(__SHORT_FILE__,__LINE__,msg) -! CPASSERT can be elided if NDEBUG is defined. -#if defined(NDEBUG) -# define CPASSERT(cond) -#else # define CPASSERT(cond) IF(.NOT.(cond))CALL cp__a(__SHORT_FILE__,__LINE__) -#endif ! The MARK_USED macro can be used to mark an argument/variable as used. It is intended to make ! it possible to switch on -Werror=unused-dummy-argument, but deal elegantly with, e.g., diff --git a/src/cp2k_info.F b/src/cp2k_info.F index 9fd7998474..571fce9b6b 100644 --- a/src/cp2k_info.F +++ b/src/cp2k_info.F @@ -85,9 +85,6 @@ CONTAINS tmp_str = "" flags = TRIM(flags)//TRIM(tmp_str) -#if defined(NDEBUG) - flags = TRIM(flags)//" ndebug" -#endif !$ flags = TRIM(flags)//" omp" #if defined(__LIBINT) flags = TRIM(flags)//" libint" diff --git a/src/environment.F b/src/environment.F index 222d8b1cb7..bbf8c90821 100644 --- a/src/environment.F +++ b/src/environment.F @@ -226,9 +226,14 @@ CONTAINS " and is therefore no longer supported.") #if !defined(__FORCE_USE_FAST_MATH) -#error -ffast-math (GCC) or -hfpN (N>0, Cray) can lead to wrong results and numerical instabilities and are therefore no longer supported +#error "-ffast-math (GCC) or -hfpN (N>0, Cray) can lead to wrong results and numerical instabilities and are therefore no longer supported" #endif #endif + +#if defined(NDEBUG) +#error "Please do not build CP2K with NDEBUG. There is no performance advantage and asserts will save your neck." +#endif + END SUBROUTINE cp2k_init ! ************************************************************************************************** diff --git a/src/grid/common/grid_library.c b/src/grid/common/grid_library.c index 2bdd67635a..1666182a1a 100644 --- a/src/grid/common/grid_library.c +++ b/src/grid/common/grid_library.c @@ -37,6 +37,11 @@ static grid_library_config config = { #error "OpenMP is required. Please add -fopenmp to your C compiler flags." #endif +#if defined(NDEBUG) +#error \ + "Please do not build CP2K with NDEBUG. There is no performance advantage and asserts will save your neck." +#endif + /******************************************************************************* * \brief Initializes the grid library. * \author Ole Schuett diff --git a/src/grid/hip/grid_hip_context.cu b/src/grid/hip/grid_hip_context.cu index 59df27dc6a..5a72496180 100644 --- a/src/grid/hip/grid_hip_context.cu +++ b/src/grid/hip/grid_hip_context.cu @@ -323,12 +323,10 @@ extern "C" void grid_hip_create_task_list( assert(num_tasks == num_tasks_per_block[i]); // check that all tasks point to the same block -#ifndef NDEBUG for (int tk = 0; tk < num_tasks; tk++) assert( tasks_host[sorted_blocks[tk + sorted_blocks_offset[i]]].block_num == i); -#endif } for (auto &block : task_sorted_by_block) block.clear(); diff --git a/tools/docker/scripts/test_regtest_cmake.sh b/tools/docker/scripts/test_regtest_cmake.sh index be5cbec37a..95b7a497ae 100755 --- a/tools/docker/scripts/test_regtest_cmake.sh +++ b/tools/docker/scripts/test_regtest_cmake.sh @@ -54,7 +54,7 @@ else echo -e "failed.\n\n" tail -n 100 ninja.log mkdir -p /workspace/artifacts/ - cp ninja.out /workspace/artifacts/ + cp ninja.log /workspace/artifacts/ echo -e "\nSummary: Compilation failed." echo -e "Status: FAILED\n" exit 0 diff --git a/tools/precommit/check_file_properties.py b/tools/precommit/check_file_properties.py index 2034b2ae37..bd47b2f3be 100755 --- a/tools/precommit/check_file_properties.py +++ b/tools/precommit/check_file_properties.py @@ -29,6 +29,7 @@ FLAG_EXCEPTIONS = ( r"LIBINT_CONTRACTED_INTS", r"XC_MAJOR_VERSION", r"XC_MINOR_VERSION", + r"NDEBUG", r"OMP_DEFAULT_NONE_WITH_OOP", r"_OPENMP", r"__COMPILE_ARCH",