Merge pull request #1498 from LLNL/feature/han12/hip_assert

Enable HIP assert in SLIC macros

Author: bmhan12

.gitlab/build_ruby.yml

@@ -7,7 +7,7 @@
 # This is the shared configuration of jobs for ruby
 .on_ruby:
   variables:
-    SCHEDULER_PARAMETERS: "--res=ci --exclusive=user --deadline=now+1hour -N1 -t ${ALLOC_TIME}"
+    SCHEDULER_PARAMETERS: "--reservation=ci --exclusive=user --deadline=now+1hour -N1 -t ${ALLOC_TIME}"
   tags:
     - batch
     - ruby

RELEASE-NOTES.md

@@ -49,6 +49,7 @@ to use Open Cascade's file I/O capabilities in support of Quest applications.
 - `MFEMSidreDataCollection::LoadExternalData` now takes two optional string parameters, one that is a
   filename (defaults to the `name` member variable) and the other is a `Group` path relative to the base of
   the Data Collection itself (defaults to the root of the `DataStore`).
+- `SLIC_ASSERT`,`SLIC_ASSERT_MSG`,`SLIC_CHECK`, and `SLIC_CHECK_MSG` macros delegate to assert() within HIP device kernels.
 
 ###  Deprecated
 

host-configs/[email protected]_hip.cmake

@@ -41,7 +41,7 @@ set(CMAKE_Fortran_FLAGS "-ef" CACHE STRING "")
 
 set(ENABLE_FORTRAN ON CACHE BOOL "")
 
-set(CMAKE_CXX_FLAGS_DEBUG "-O1 -g -DNDEBUG" CACHE STRING "")
+set(CMAKE_CXX_FLAGS_DEBUG "-O1 -g" CACHE STRING "")
 
 #------------------------------------------------------------------------------
 # MPI

host-configs/[email protected]_hip.cmake

@@ -41,8 +41,6 @@ set(CMAKE_Fortran_FLAGS "-Mfreeform" CACHE STRING "")
 
 set(ENABLE_FORTRAN ON CACHE BOOL "")
 
-set(CMAKE_CXX_FLAGS_DEBUG "-O1 -g -DNDEBUG" CACHE STRING "")
-
 #------------------------------------------------------------------------------
 # MPI
 #------------------------------------------------------------------------------

host-configs/[email protected]_hip.cmake

@@ -41,8 +41,6 @@ set(CMAKE_Fortran_FLAGS "-Mfreeform" CACHE STRING "")
 
 set(ENABLE_FORTRAN ON CACHE BOOL "")
 
-set(CMAKE_CXX_FLAGS_DEBUG "-O1 -g -DNDEBUG" CACHE STRING "")
-
 #------------------------------------------------------------------------------
 # MPI
 #------------------------------------------------------------------------------

host-configs/[email protected]_hip.cmake

@@ -41,7 +41,7 @@ set(CMAKE_Fortran_FLAGS "-ef" CACHE STRING "")
 
 set(ENABLE_FORTRAN ON CACHE BOOL "")
 
-set(CMAKE_CXX_FLAGS_DEBUG "-O1 -g -DNDEBUG" CACHE STRING "")
+set(CMAKE_CXX_FLAGS_DEBUG "-O1 -g" CACHE STRING "")
 
 #------------------------------------------------------------------------------
 # MPI

host-configs/[email protected]_hip.cmake

@@ -41,8 +41,6 @@ set(CMAKE_Fortran_FLAGS "-Mfreeform" CACHE STRING "")
 
 set(ENABLE_FORTRAN ON CACHE BOOL "")
 
-set(CMAKE_CXX_FLAGS_DEBUG "-O1 -g -DNDEBUG" CACHE STRING "")
-
 #------------------------------------------------------------------------------
 # MPI
 #------------------------------------------------------------------------------

host-configs/[email protected]_hip.cmake

@@ -41,8 +41,6 @@ set(CMAKE_Fortran_FLAGS "-Mfreeform" CACHE STRING "")
 
 set(ENABLE_FORTRAN ON CACHE BOOL "")
 
-set(CMAKE_CXX_FLAGS_DEBUG "-O1 -g -DNDEBUG" CACHE STRING "")
-
 #------------------------------------------------------------------------------
 # MPI
 #------------------------------------------------------------------------------

scripts/spack/packages/axom/package.py

@@ -288,9 +288,9 @@ def initconfig_compiler_entries(self):
         if "+cpp14" in spec and spec.satisfies("@:0.6.1"):
             entries.append(cmake_cache_string("BLT_CXX_STD", "c++14", ""))
 
-        # Add optimization flag workaround for Debug builds with cray compiler or newer HIP
-        if "+rocm" in spec:
-            entries.append(cmake_cache_string("CMAKE_CXX_FLAGS_DEBUG", "-O1 -g -DNDEBUG"))
+        # Add optimization flag workaround for builds with cray compiler
+        if spec.satisfies("%cce"):
+            entries.append(cmake_cache_string("CMAKE_CXX_FLAGS_DEBUG", "-O1 -g"))
 
         return entries
 

src/axom/core/Macros.hpp

@@ -15,6 +15,11 @@
 #include "axom/config.hpp"
 #include <cassert>  // for assert()
 
+// Header for assert() in HIP device kernels
+#if defined(AXOM_USE_HIP)
+  #include <hip/hip_runtime.h>
+#endif
+
 // _guarding_macros_start
 /*!
  * \def AXOM_USE_GPU

src/axom/slic/interface/slic_macros.hpp

@@ -491,59 +491,15 @@
 
 /// @}
 
-// Use assert when on device (HIP does not yet support assert())
+// Use assert when on device (note that messages are omitted).
+// Device HIP assert() tested with [email protected]
+// (ROCm support for device assert() begins with version 5.1.0).
 #elif defined(AXOM_DEBUG) && defined(AXOM_DEVICE_CODE)
-  #if !defined(__HIP_DEVICE_COMPILE__)
-    #define SLIC_ASSERT(EXP) assert(EXP)
-    #define SLIC_ASSERT_MSG(EXP, msg) assert(EXP)
-    #define SLIC_CHECK(EXP) assert(EXP)
-    #define SLIC_CHECK_MSG(EXP, msg) assert(EXP)
-  #else
-// AXOM_DEBUG is defined so the expression and message cannot be ignored
-// as they probably contain parameters that were decorated with AXOM_UNUSED_PARAM
-// which will require the expression and message to be used to avoid warnings.
-// Here we use them in a way that will not really have a runtime effect.
-// The msg code block may contain << operators so we need a stream-like object
-// but we enclose it in "if(false)" so it never executes.
-namespace axom
-{
-namespace slic
-{
-namespace internal
-{
-class blackhole
-{ };
-// Write an object to a blackhole.
-template <typename T>
-AXOM_HOST_DEVICE inline blackhole &operator<<(blackhole &bh, T)
-{
-  return bh;
-}
-}  // namespace internal
-}  // namespace slic
-}  // namespace axom
-
-    #define SLIC_ASSERT(EXP) ((void)(EXP))
-    #define SLIC_ASSERT_MSG(EXP, msg)            \
-      {                                          \
-        if(false)                                \
-        {                                        \
-          ((void)(EXP));                         \
-          axom::slic::internal::blackhole __oss; \
-          __oss << msg;                          \
-        }                                        \
-      }
-    #define SLIC_CHECK(EXP) ((void)(EXP))
-    #define SLIC_CHECK_MSG(EXP, msg)             \
-      {                                          \
-        if(false)                                \
-        {                                        \
-          ((void)(EXP));                         \
-          axom::slic::internal::blackhole __oss; \
-          __oss << msg;                          \
-        }                                        \
-      }
-  #endif
+  #define SLIC_ASSERT(EXP) assert(EXP)
+  #define SLIC_ASSERT_MSG(EXP, msg) assert(EXP)
+  #define SLIC_CHECK(EXP) assert(EXP)
+  #define SLIC_CHECK_MSG(EXP, msg) assert(EXP)
+
 #else  // turn off debug macros and asserts
 
   #define SLIC_ASSERT(ignore_EXP) ((void)0)

src/tools/CMakeLists.txt

@@ -83,20 +83,7 @@ if(AXOM_ENABLE_QUEST)
         set (_policies "raja_seq")
         blt_list_append(TO _policies ELEMENTS "raja_omp" IF AXOM_ENABLE_OPENMP)
         blt_list_append(TO _policies ELEMENTS "raja_cuda" IF AXOM_ENABLE_CUDA)
-
-        # Test with HIP policy for select optimization flags.
-        # Check for Debug flags that have been overwritten.
-        if(AXOM_ENABLE_HIP)
-            if((CMAKE_BUILD_TYPE MATCHES "(Release|RelWithDebInfo)") OR
-              (CMAKE_BUILD_TYPE MATCHES "Debug" AND
-              CMAKE_CXX_FLAGS_DEBUG MATCHES "\-O1 \-g \-DNDEBUG"))
-                list(APPEND _policies "raja_hip")
-            else()
-              message(STATUS
-                "HIP policy for mesh_tester executable is not being tested"
-              )
-            endif()
-        endif()
+        blt_list_append(TO _policies ELEMENTS "raja_hip" IF AXOM_ENABLE_HIP)
 
         foreach(_method ${_methods})
             foreach(_policy ${_policies})

src/tools/mesh_tester.cpp

@@ -51,7 +51,7 @@ using seq_exec = axom::SEQ_EXEC;
     using cuda_exec = seq_exec;
   #endif
 
-  #if defined(AXOM_USE_HIP) && defined(AXOM_USE_UMPIRE) && defined(NDEBUG)
+  #if defined(AXOM_USE_HIP) && defined(AXOM_USE_UMPIRE)
     constexpr int HIP_BLOCK_SIZE = 256;
     using hip_exec = axom::HIP_EXEC<HIP_BLOCK_SIZE>;
   #else
@@ -139,7 +139,7 @@ const std::map<std::string, RuntimePolicy> Input::s_validPolicies({
     #ifdef AXOM_USE_CUDA
   , {"raja_cuda", raja_cuda}
     #endif
-    #if defined(AXOM_USE_HIP) && defined(NDEBUG)
+    #if defined(AXOM_USE_HIP)
   , {"raja_hip", raja_hip}
     #endif
   #endif
@@ -181,7 +181,7 @@ void Input::parse(int argc, char** argv, axom::CLI::App& app)
   #ifdef AXOM_USE_CUDA
   pol_sstr << "\nSet to 'raja_cuda' or 3 to use the RAJA CUDA policy.";
   #endif
-  #if defined(AXOM_USE_HIP) && defined(NDEBUG)
+  #if defined(AXOM_USE_HIP)
   pol_sstr << "\nSet to 'raja_hip' or 4 to use the RAJA HIP policy.";
   #endif
 #endif
@@ -705,7 +705,7 @@ int main(int argc, char** argv)
                                                 params.intersectionThreshold);
         break;
   #endif
-  #if defined(AXOM_USE_HIP) && defined(NDEBUG)
+  #if defined(AXOM_USE_HIP)
       case raja_hip:
         collisions =
           naiveIntersectionAlgorithm<hip_exec>(surface_mesh,
@@ -762,7 +762,7 @@ int main(int argc, char** argv)
           params.intersectionThreshold);
         break;
   #endif
-  #if defined(AXOM_USE_HIP) && defined(NDEBUG)
+  #if defined(AXOM_USE_HIP)
       case raja_hip:
         quest::findTriMeshIntersectionsBVH<hip_exec, double>(
           surface_mesh,
@@ -823,7 +823,7 @@ int main(int argc, char** argv)
           params.intersectionThreshold);
         break;
   #endif
-  #if defined(AXOM_USE_HIP) && defined(AXOM_USE_UMPIRE) && defined(NDEBUG)
+  #if defined(AXOM_USE_HIP) && defined(AXOM_USE_UMPIRE)
       case raja_hip:
         quest::findTriMeshIntersectionsImplicitGrid<hip_exec, double>(
           surface_mesh,
@@ -882,7 +882,7 @@ int main(int argc, char** argv)
           params.intersectionThreshold);
         break;
   #endif
-  #if defined(AXOM_USE_HIP) && defined(AXOM_USE_UMPIRE) && defined(NDEBUG)
+  #if defined(AXOM_USE_HIP) && defined(AXOM_USE_UMPIRE)
       case raja_hip:
         quest::findTriMeshIntersectionsUniformGrid<hip_exec, double>(
           surface_mesh,