Fix Dynamic memory space when configured with Umpire

RELEASE-NOTES.md

@@ -20,6 +20,9 @@ The Axom project release numbers follow [Semantic Versioning](http://semver.org/
 ## [Unreleased] - Release date yyyy-mm-dd
 
 ###  Added
+- New `axom::MALLOC_ALLOCATOR_ID` is for using malloc and free.
+  This id uses malloc/free on the host, and works even when Axom is
+  configured with Umpire.  Non-Umpire allocator ids are negative.
 - Added a new "Mir" Axom component for accelerated Material Interface Reconstruction (MIR) algorithms.
   MIR algorithms take Blueprint meshes with a matset and they use the matset's material information
   to split any input zones that contain multiple materials into zones that contain a single material.
@@ -43,6 +46,8 @@ to use Open Cascade's file I/O capabilities in support of Quest applications.
 - Adds intersection routines between `primal::Ray` objects and `primal::NURBSCurve`/`primal::NURBSPatch` objects.
 
 ###  Changed
+- The default allocator id zero in `memoryh_management.hpp` is now in the
+  `axom::DYNAMIC_ALLOCATOR_ID` and has a different value.
 - `primal::NumericArray` has been moved to `core`.  The header is `core/NumericArray.hpp`.
 - `quest::Shaper` and `quest::IntersectionShaper` constructors require a runtime policy.
   Changing the policy after construction is no longer supported.

src/axom/core/memory_management.hpp

@@ -9,6 +9,7 @@
 // Axom includes
 #include "axom/config.hpp"  // for AXOM compile-time definitions
 #include "axom/core/Macros.hpp"
+#include "axom/core/utilities/Utilities.hpp"
 
 // Umpire includes
 #ifdef AXOM_USE_UMPIRE
@@ -22,10 +23,14 @@
   #include <cstdlib>  // for std::malloc, std::realloc, std::free
 #endif
 
+#include <iostream>
+
 namespace axom
 {
+// To co-exist with Umpire allocator ids, use negative values here.
 constexpr int INVALID_ALLOCATOR_ID = -1;
-constexpr int DYNAMIC_ALLOCATOR_ID = 0;
+constexpr int DYNAMIC_ALLOCATOR_ID = -2;
+constexpr int MALLOC_ALLOCATOR_ID = -3;
 
 // _memory_space_start
 /*! 
@@ -38,6 +43,7 @@ constexpr int DYNAMIC_ALLOCATOR_ID = 0;
 enum class MemorySpace
 {
   Dynamic,
+  Malloc,
 #ifdef AXOM_USE_UMPIRE
   Host,
   Device,
@@ -115,7 +121,7 @@ inline int getDefaultAllocatorID()
  * \brief Get the allocator id from which data has been allocated.
  * \return Allocator id.  If Umpire doesn't have an allocator for
  * the pointer, or Axom wasn't configured with Umpire, return
- * \c axom::DYNAMIC_ALLOCATOR_ID.
+ * \c axom::MALLOC_ALLOCATOR_ID.
  *
  * \pre ptr has a valid pointer value.
  */
@@ -129,6 +135,8 @@ inline int getAllocatorIDFromPointer(const void* ptr)
     return allocator.getId();
   }
 #endif
+  // TODO: How to distinguish between dynamic and malloc allocators?
+  // Can they coexist?
   AXOM_UNUSED_VAR(ptr);
   return DYNAMIC_ALLOCATOR_ID;
 }
@@ -210,15 +218,29 @@ inline T* allocate(std::size_t n, int allocID) noexcept
   const std::size_t numbytes = n * sizeof(T);
 
 #ifdef AXOM_USE_UMPIRE
-
   umpire::ResourceManager& rm = umpire::ResourceManager::getInstance();
-  umpire::Allocator allocator = rm.getAllocator(allocID);
-  return static_cast<T*>(allocator.allocate(numbytes));
-
-#else
-  AXOM_UNUSED_VAR(allocID);
-  return static_cast<T*>(std::malloc(numbytes));
+  if(rm.isAllocator(allocID))
+  {
+    umpire::Allocator allocator = rm.getAllocator(allocID);
+    return static_cast<T*>(allocator.allocate(numbytes));
+  }
 #endif
+
+  if(allocID == MALLOC_ALLOCATOR_ID)
+  {
+    return static_cast<T*>(std::malloc(numbytes));
+  }
+
+  if(allocID == DYNAMIC_ALLOCATOR_ID)
+  {
+    // TODO: Is this the correct behavior for Dynamic?
+    return static_cast<T*>(std::malloc(numbytes));
+  }
+
+  std::cerr << "Unrecognized allocator id " << allocID << std::endl;
+  axom::utilities::processAbort();
+
+  return nullptr;  // Silence warning.
 }
 //------------------------------------------------------------------------------
 template <typename T>
@@ -232,46 +254,81 @@ inline void deallocate(T*& pointer) noexcept
 #ifdef AXOM_USE_UMPIRE
 
   umpire::ResourceManager& rm = umpire::ResourceManager::getInstance();
-  rm.deallocate(pointer);
-
-#else
-
-  std::free(pointer);
+  if(rm.hasAllocator(pointer))
+  {
+    rm.deallocate(pointer);
+    pointer = nullptr;
+    return;
+  }
 
 #endif
 
+  std::free(pointer);
   pointer = nullptr;
 }
 
 //------------------------------------------------------------------------------
 template <typename T>
 inline T* reallocate(T* pointer, std::size_t n, int allocID) noexcept
 {
+  assert(allocID != INVALID_ALLOCATOR_ID);
+
   const std::size_t numbytes = n * sizeof(T);
 
 #if defined(AXOM_USE_UMPIRE)
 
   umpire::ResourceManager& rm = umpire::ResourceManager::getInstance();
-  if(pointer == nullptr)
-  {
-    pointer = axom::allocate<T>(n, allocID);
-  }
-  else
+  if(rm.isAllocator(allocID))
   {
-    pointer = static_cast<T*>(rm.reallocate(pointer, numbytes));
+    if(pointer == nullptr)
+    {
+      pointer = axom::allocate<T>(n, allocID);
+    }
+    else
+    {
+      if(rm.hasAllocator(pointer))
+      {
+        pointer = static_cast<T*>(rm.reallocate(pointer, numbytes));
+      }
+      else
+      {
+        /*
+          Reallocate from non-Umpire to Umpire, manually, using
+          allocate, copy and deallocate.  Because we don't know the
+          current size, we first do a (extra) reallocate within the
+          current space just so we have the size for the copy.
+          Is there a better way?
+        */
+        auto tmpPointer = std::realloc(pointer, numbytes);
+        pointer = axom::allocate<T>(n, allocID);
+        copy(pointer, tmpPointer, numbytes);
+        deallocate(tmpPointer);
+      }
+    }
+    return pointer;
   }
 
 #else
 
-  pointer = static_cast<T*>(std::realloc(pointer, numbytes));
+  if(allocID == MALLOC_ALLOCATOR_ID || allocID == DYNAMIC_ALLOCATOR_ID)
+  {
+    pointer = static_cast<T*>(std::realloc(pointer, numbytes));
+  }
+  else
+  {
+    std::cerr << "*** Unrecognized allocator id " << allocID <<
+              ".  Axom was NOT built with Umpire, so the only valid allocator ids are MALLOC_ALLOCATOR_ID ("
+              << MALLOC_ALLOCATOR_ID << ") and DYNAMIC_ALLOCATOR_ID (" << DYNAMIC_ALLOCATOR_ID << ")."
+              << std::endl;
+    axom::utilities::processAbort();
+  }
 
   // Consistently handle realloc(0) for std::realloc to match Umpire's behavior
   if(n == 0 && pointer == nullptr)
   {
     pointer = axom::allocate<T>(0);
   }
 
-  AXOM_UNUSED_VAR(allocID);
 #endif
 
   return pointer;
@@ -330,6 +387,7 @@ inline MemorySpace getAllocatorSpace(int allocatorId)
   // throws exception if given a non-Umpire id.
   assert(allocatorId != INVALID_ALLOCATOR_ID);
   if(allocatorId == DYNAMIC_ALLOCATOR_ID) return MemorySpace::Dynamic;
+  if(allocatorId == MALLOC_ALLOCATOR_ID) return MemorySpace::Malloc;
 
 #ifdef AXOM_USE_UMPIRE
   using ump_res_type = typename umpire::MemoryResourceTraits::resource_type;
@@ -354,6 +412,8 @@ inline MemorySpace getAllocatorSpace(int allocatorId)
     return MemorySpace::Dynamic;
   }
 #endif
+
+  return MemorySpace::Dynamic;  // Silence warning.
 }
 
 #ifdef AXOM_USE_UMPIRE

src/axom/core/tests/core_memory_management.hpp

@@ -506,3 +506,72 @@ TEST(core_memory_management, allocator_id_from_pointer)
   delete[] buf;
 }
 #endif
+
+//------------------------------------------------------------------------------
+TEST(core_memory_management, interspace_reallocation)
+{
+  // Allocator ids to test.
+  std::vector<int> allocIds(1, axom::MALLOC_ALLOCATOR_ID);
+#ifdef AXOM_USE_UMPIRE
+  allocIds.push_back(axom::detail::getAllocatorID<axom::MemorySpace::Host>());
+  #ifdef AXOM_USE_GPU
+  allocIds.push_back(axom::detail::getAllocatorID<axom::MemorySpace::Device>());
+  allocIds.push_back(axom::detail::getAllocatorID<axom::MemorySpace::Unified>());
+    // Does it make sense to check Pinned and Constant memory spaces?
+  #endif
+#endif
+
+  // We'll allocate N items, reallocate to K items, reallocate back to N.
+  constexpr std::size_t N = 5;
+  constexpr std::size_t K = 8;
+  constexpr std::size_t maxNK = std::max(N, K);
+  constexpr std::size_t minNK = std::min(N, K);
+
+  // origOnHost and tempOnHost are for initialization and results-checking on host.
+  int* origOnHost = axom::allocate<int>(maxNK, axom::MALLOC_ALLOCATOR_ID);
+  for(std::size_t i = 0; i < maxNK; ++i)
+  {
+    origOnHost[i] = 100 + i;
+  }
+  int* tempOnHost = axom::allocate<int>(maxNK, axom::MALLOC_ALLOCATOR_ID);
+
+  // Count differences between origOnHost and tempOnHost.
+  auto countDiffs = [=]() {
+    std::size_t diffCount = 0;
+    for(std::size_t j = 0; j < minNK; ++j)
+    {
+      diffCount += tempOnHost[j] != origOnHost[j];
+    }
+    return diffCount;
+  };
+
+  std::size_t diffCount = 0;
+  for(auto srcAllocId : allocIds)
+  {
+    for(auto dstAllocId : allocIds)
+    {
+      std::cout << "Testing allocator ids " << srcAllocId << " and "
+                << dstAllocId << std::endl;
+      // For each combination of srcAllocId and dstAllocId,
+      // allocate src, reallocate to dst, reallocate back to src.
+
+      int* src = axom::allocate<int>(N, srcAllocId);
+      axom::copy(src, origOnHost, N * sizeof(int));
+
+      int* dst = axom::reallocate(src, K, dstAllocId);
+      axom::copy(tempOnHost, dst, N * sizeof(int));
+      diffCount = countDiffs();
+      EXPECT_EQ(diffCount, 0);
+
+      src = axom::reallocate(dst, N, srcAllocId);
+      axom::copy(tempOnHost, src, N * sizeof(int));
+      diffCount = countDiffs();
+      EXPECT_EQ(diffCount, 0);
+
+      axom::deallocate(src);
+    }
+  }
+
+  axom::deallocate(origOnHost);
+  axom::deallocate(tempOnHost);
+}

src/thirdparty/tests/CMakeLists.txt

@@ -39,7 +39,7 @@ if ( RAJA_FOUND )
   axom_add_executable(NAME raja_smoke_test
                       SOURCES raja_smoke.cpp
                       OUTPUT_DIR ${TEST_OUTPUT_DIRECTORY}
-                      DEPENDS_ON ${raja_smoke_dependencies}
+                      DEPENDS_ON ${raja_smoke_dependencies} core
                       FOLDER axom/thirdparty/tests )
 
   target_include_directories(raja_smoke_test PUBLIC