Fixup issues

Author: davidbeckingsale

CMakeLists.txt

@@ -203,7 +203,7 @@ if (UMPIRE_ENABLE_TESTS)
   add_subdirectory(tests)
 endif ()
 
-if (UMPIRE_ENABLE_BENCHMARKS)
+if (UMPIRE_ENABLE_BENCHMARKS OR UMPIRE_ENABLE_DEVELOPER_BENCHMARKS)
   add_subdirectory(benchmarks)
   if ((NOT CMAKE_BUILD_TYPE) OR (NOT ${CMAKE_BUILD_TYPE} STREQUAL "Release"))
     message("-- Warning: CMAKE_BUILD_TYPE not set to Release, benchmark information will not be reliable for this build!")

benchmarks/CMakeLists.txt

@@ -23,6 +23,10 @@ if (UMPIRE_ENABLE_CUDA)
     SOURCES resource_aware_pool_stress_test.cpp
     DEPENDS_ON umpire cuda)
 
+  blt_add_target_compile_flags(
+    TO resource_aware_pool_stress_test
+    FLAGS ${UMPIRE_DISABLE_DEPRECATED_WARNINGS_FLAG})
+
   if (UMPIRE_ENABLE_DEVICE_ALLOCATOR)
   blt_add_executable(
     NAME device_allocator_stress_test
@@ -43,6 +47,10 @@ elseif (UMPIRE_ENABLE_HIP)
     SOURCES resource_aware_pool_stress_test.cpp
     DEPENDS_ON umpire blt::hip blt::hip_runtime)
 
+  blt_add_target_compile_flags(
+    TO resource_aware_pool_stress_test
+    FLAGS ${UMPIRE_DISABLE_DEPRECATED_WARNINGS_FLAG})
+
   if (UMPIRE_ENABLE_DEVICE_ALLOCATOR)
     blt_add_executable(
       NAME device_allocator_stress_test
@@ -65,6 +73,10 @@ blt_add_executable(
   SOURCES copy_stress_test.cpp
   DEPENDS_ON ${stress_test_depends})
 
+blt_add_target_compile_flags(
+  TO copy_stress_test
+  FLAGS ${UMPIRE_DISABLE_DEPRECATED_WARNINGS_FLAG})
+
 blt_add_executable(
   NAME fixed_pool_stress_test
   SOURCES fixed_pool_stress_test.cpp
@@ -135,6 +147,10 @@ blt_add_executable(
   SOURCES copy_benchmarks.cpp
   DEPENDS_ON ${benchmark_depends})
 
+blt_add_target_compile_flags(
+  TO copy_benchmarks
+  FLAGS ${UMPIRE_DISABLE_DEPRECATED_WARNINGS_FLAG})
+
 blt_add_benchmark(
   NAME copy_benchmarks
   COMMAND copy_benchmarks)

docs/doxygen/Doxyfile.in

@@ -448,7 +448,7 @@ WARN_LOGFILE           = dox.warnings
 # directories like "/usr/src/myproject". Separate the files or directories 
 # with spaces.
 
-INPUT                  = @PROJECT_SOURCE_DIR@/src
+INPUT                  = @PROJECT_SOURCE_DIR@/src @PROJECT_SOURCE_DIR@/include
 
 # Allow Markdown files to provide the Doxygen main page
 

docs/sphinx/getting_started.rst

@@ -105,13 +105,13 @@ allocate memory in different places. Let's ask for a device allocator:
 
 This code gets the default device allocator, and uses it to allocate an array
 of 100 floats. Remember, since this is a device pointer, there is no guarantee
-you will be able to access it on the host.  Luckily, Umpire's ResourceManager
-can copy one pointer to another transparently. Let's copy the data from our
-first pointer to the DEVICE-allocated pointer.
+you will be able to access it on the host.  Luckily, Umpire provides a
+``copy`` function that can copy one pointer to another transparently. Let's
+copy the data from our first pointer to the DEVICE-allocated pointer.
 
 .. code-block:: cpp
 
-  rm.copy(my_data, my_data_device);
+  umpire::copy(my_data, my_data_device, 100);
 
 To free any memory allocated, you can use the deallocate function of the
 Allocator, or the ResourceManager. Asking the ResourceManager to deallocate

examples/tutorial/tut_memset.cpp

@@ -38,7 +38,7 @@ int main(int, char**)
               << std::endl;
 
     // _sphinx_tag_tut_memset_start
-    umpire::memset(data, 0, SIZE * sizeof(double));
+    umpire::memset(data, 0, SIZE);
     // _sphinx_tag_tut_memset_end
 
     std::cout << "Set data from " << destination << " (" << data << ") to 0." << std::endl;

include/umpire/op/cuda.hpp

@@ -89,6 +89,10 @@ inline cudaStream_t get_stream(camp::resources::Resource& resource)
 /**
  * @brief Apply memory advice to a CUDA managed memory allocation
  *
+ * Advise is a performance hint, not a correctness requirement, so this is
+ * intentionally a no-op (with a logged warning) on devices that do not
+ * support managed memory.
+ *
  * @tparam T Type of memory
  * @param ptr Pointer to memory
  * @param count Number of elements
@@ -99,11 +103,18 @@ template <typename T>
 inline void advise(T* ptr, std::size_t count, int device, cudaMemoryAdvise advice)
 {
   // Skip if device doesn't support managed memory
-  if (!supports_managed_memory(device))
+  if (!supports_managed_memory(device)) {
+    UMPIRE_LOG(Warning, "cudaMemAdvise skipped: device " << device << " does not support managed memory");
     return;
+  }
 
   std::size_t size = detail::get_size<T>(count);
+#if CUDART_VERSION >= 13000
+  cudaMemLocation loc = {(device == cudaCpuDeviceId) ? cudaMemLocationTypeHost : cudaMemLocationTypeDevice, device};
+  cudaError_t error = ::cudaMemAdvise(ptr, size, advice, loc);
+#else
   cudaError_t error = ::cudaMemAdvise(ptr, size, advice, device);
+#endif
 
   if (error != cudaSuccess) {
     UMPIRE_ERROR(runtime_error,
@@ -228,10 +239,17 @@ inline void prefetch(T* ptr, int device, std::size_t count)
                  fmt::format("cudaGetDevice failed: {} ({})", cudaGetErrorString(get_dev_err), get_dev_err));
   }
   int gpu = (device != cudaCpuDeviceId) ? device : current_device;
+#if CUDART_VERSION >= 13000
+  cudaMemLocation loc = {(device == cudaCpuDeviceId) ? cudaMemLocationTypeHost : cudaMemLocationTypeDevice, device};
+#endif
 
   if (supports_managed_memory(gpu)) {
     std::size_t size = detail::get_size<T>(count);
+#if CUDART_VERSION >= 13000
+    cudaError_t error = ::cudaMemPrefetchAsync(ptr, size, loc, 0, nullptr);
+#else
     cudaError_t error = ::cudaMemPrefetchAsync(ptr, size, device, nullptr);
+#endif
 
     if (error != cudaSuccess) {
       UMPIRE_ERROR(runtime_error, fmt::format("cudaMemPrefetchAsync(ptr={}, size={}, device={}) failed with error: {}",
@@ -264,10 +282,17 @@ inline camp::resources::EventProxy<camp::resources::Resource> prefetch_async(T*
                  fmt::format("cudaGetDevice failed: {} ({})", cudaGetErrorString(get_dev_err), get_dev_err));
   }
   int gpu = (device != cudaCpuDeviceId) ? device : current_device;
+#if CUDART_VERSION >= 13000
+  cudaMemLocation loc = {(device == cudaCpuDeviceId) ? cudaMemLocationTypeHost : cudaMemLocationTypeDevice, device};
+#endif
 
   if (supports_managed_memory(gpu)) {
     std::size_t size = detail::get_size<T>(count);
+#if CUDART_VERSION >= 13000
+    cudaError_t error = ::cudaMemPrefetchAsync(ptr, size, loc, 0, stream);
+#else
     cudaError_t error = ::cudaMemPrefetchAsync(ptr, size, device, stream);
+#endif
 
     if (error != cudaSuccess) {
       UMPIRE_ERROR(runtime_error,

include/umpire/op/detail/utils.hpp

@@ -1,27 +1,35 @@
 #pragma once
 
 #include <cstddef>
+#include <limits>
 #include <type_traits>
 #include "camp/resource.hpp"
+#include "umpire/util/error.hpp"
 
 namespace umpire {
 namespace op {
 namespace detail {
 
 /**
  * @brief Calculate size in bytes based on element count and type
- * 
+ *
  * @tparam T The pointer type (void* or typed pointer)
  * @param count Number of elements or bytes (if T is void)
  * @return std::size_t Size in bytes
  */
 template <typename T>
-inline std::size_t get_size(std::size_t count) noexcept
+inline std::size_t get_size(std::size_t count)
 {
-  if constexpr (std::is_same_v<T, void>)
+  if constexpr (std::is_same_v<T, void>) {
     return count;
-  else
+  } else {
+    if (count > (std::numeric_limits<std::size_t>::max() / sizeof(T))) {
+      UMPIRE_ERROR(runtime_error,
+                   fmt::format("Requested size overflow: count ({}) * sizeof(T) ({}) exceeds size_t max", count,
+                               sizeof(T)));
+    }
     return count * sizeof(T);
+  }
 }
 
 /**

include/umpire/op/dispatch.hpp

@@ -6,6 +6,7 @@
 #include "umpire/ResourceManager.hpp"
 #include "umpire/config.hpp"
 #include "umpire/op/detail/traits.hpp"
+#include "umpire/op/detail/utils.hpp"
 #include "umpire/resource/platform.hpp"
 
 namespace umpire {
@@ -386,9 +387,27 @@ struct op_caller {
 //
 // This keeps the API consistent with C++ idioms where typed operations work
 // with element counts and void* operations work with byte counts.
+//
+// *** ARGUMENT ORDER WARNING ***
+//   umpire::copy(src, dst, ...) takes (SOURCE, DESTINATION) order.
+//   This is the REVERSE of the deprecated ResourceManager::copy(dst_ptr, src_ptr, ...),
+//   which takes (DESTINATION, SOURCE) order.
+//
+//   A mechanical migration from ResourceManager::copy() to umpire::copy() that does
+//   NOT swap the two pointer arguments will compile cleanly (both overloads accept
+//   the same pointer type) and will silently copy data in the WRONG DIRECTION --
+//   there is no compiler error or warning to catch this mistake. Callers migrating
+//   from ResourceManager::copy(dst, src, ...) MUST swap to umpire::copy(src, dst, ...).
+//
+//   Also note the size-parameter semantics above still apply: the typed
+//   umpire::copy<T>(src, dst, len) overloads take len as an ELEMENT COUNT, while the
+//   void* umpire::copy(src, dst, len) overload takes len as a BYTE COUNT.
 //------------------------------------------------------------------------------
 
 // Global operation implementations that use the op_caller
+//
+// NOTE: Argument order is (src, dst) -- the REVERSE of the deprecated
+// ResourceManager::copy(dst, src, ...). See the "ARGUMENT ORDER WARNING" above.
 template <typename T>
 void copy(T* src, T* dst, std::size_t len)
 {
@@ -524,12 +543,7 @@ PtrType* op::reallocate<Platform>::reallocate_impl_sync(PtrType** ptr_ptr, std::
     auto& rm = ResourceManager::getInstance();
     Allocator allocator = rm.getDefaultAllocator();
 
-    std::size_t alloc_bytes;
-    if constexpr (std::is_same_v<PtrType, void>) {
-      alloc_bytes = new_size; // Void: new_size is already in bytes
-    } else {
-      alloc_bytes = new_size * sizeof(PtrType); // Typed: new_size is element count
-    }
+    std::size_t alloc_bytes = detail::get_size<PtrType>(new_size);
 
     PtrType* new_ptr = static_cast<PtrType*>(allocator.allocate(alloc_bytes));
     *ptr_ptr = new_ptr;
@@ -550,13 +564,7 @@ PtrType* op::reallocate<Platform>::reallocate_impl_sync(PtrType** ptr_ptr, std::
 
   // 4. Calculate sizes
   std::size_t old_size = rm.getSize(current_ptr);
-  std::size_t new_bytes;
-
-  if constexpr (std::is_same_v<PtrType, void>) {
-    new_bytes = new_size; // Void: new_size is already in bytes
-  } else {
-    new_bytes = new_size * sizeof(PtrType); // Typed: new_size is element count
-  }
+  std::size_t new_bytes = detail::get_size<PtrType>(new_size);
 
   // 5. Zero-byte special case
   if (new_bytes == 0) {
@@ -603,12 +611,7 @@ camp::resources::EventProxy<camp::resources::Resource> op::reallocate<Platform>:
     auto& rm = ResourceManager::getInstance();
     Allocator allocator = rm.getDefaultAllocator();
 
-    std::size_t alloc_bytes;
-    if constexpr (std::is_same_v<PtrType, void>) {
-      alloc_bytes = new_size; // Void: new_size is already in bytes
-    } else {
-      alloc_bytes = new_size * sizeof(PtrType); // Typed: new_size is element count
-    }
+    std::size_t alloc_bytes = detail::get_size<PtrType>(new_size);
 
     PtrType* new_ptr = static_cast<PtrType*>(allocator.allocate(alloc_bytes));
     *ptr_ptr = new_ptr;
@@ -629,13 +632,7 @@ camp::resources::EventProxy<camp::resources::Resource> op::reallocate<Platform>:
 
   // 4. Calculate sizes
   std::size_t old_size = rm.getSize(current_ptr);
-  std::size_t new_bytes;
-
-  if constexpr (std::is_same_v<PtrType, void>) {
-    new_bytes = new_size; // Void: new_size is already in bytes
-  } else {
-    new_bytes = new_size * sizeof(PtrType); // Typed: new_size is element count
-  }
+  std::size_t new_bytes = detail::get_size<PtrType>(new_size);
 
   // 5. Zero-byte special case
   if (new_bytes == 0) {
@@ -648,6 +645,13 @@ camp::resources::EventProxy<camp::resources::Resource> op::reallocate<Platform>:
   // 6. Allocate new memory
   PtrType* new_ptr = static_cast<PtrType*>(allocator.allocate(new_bytes));
 
+  // Guard new pointer for exception safety: if the copy below throws (or the
+  // wait throws), release the new allocation so it is not leaked and *ptr_ptr
+  // is not left dangling.
+  auto new_cleanup = detail::make_scope_exit([&]() {
+    allocator.deallocate(new_ptr);
+  });
+
   // 7. Copy data asynchronously
   std::size_t copy_bytes = (old_size > new_bytes) ? new_bytes : old_size;
 
@@ -658,10 +662,16 @@ camp::resources::EventProxy<camp::resources::Resource> op::reallocate<Platform>:
     allocator.deallocate(current_ptr);
   });
 
-  // Wait for async copy to complete before deallocating to avoid race condition
+  // NOTE: This wait is an intentional blocking point. It guarantees the copy
+  // has completed before the old buffer is freed (and before we dismiss the
+  // new-pointer guard below). A true non-blocking async reallocate that defers
+  // the old-buffer free until the event completes is left as a follow-up.
   static_cast<camp::resources::Event>(event).wait();
   // cleanup happens automatically via RAII
 
+  // Copy (and wait) succeeded: the new allocation is valid, dismiss its guard.
+  new_cleanup.dismiss();
+
   *ptr_ptr = new_ptr;
   return event;
 }

include/umpire/op/hip.hpp

@@ -86,6 +86,10 @@ inline hipStream_t get_stream(camp::resources::Resource& resource)
 /**
  * @brief Apply memory advice to a HIP managed memory allocation
  *
+ * @note advise() is a performance hint, not a correctness requirement. On
+ * devices that do not support managed memory, this is intentionally a
+ * logged no-op rather than an error.
+ *
  * @tparam T Type of memory
  * @param ptr Pointer to memory
  * @param count Number of elements
@@ -96,8 +100,10 @@ template <typename T>
 inline void advise(T* ptr, std::size_t count, int device, hipMemoryAdvise advice)
 {
   // Skip if device doesn't support managed memory
-  if (!supports_managed_memory(device))
+  if (!supports_managed_memory(device)) {
+    UMPIRE_LOG(Warning, "hipMemAdvise skipped: device " << device << " does not support managed memory");
     return;
+  }
 
   std::size_t size = detail::get_size<T>(count);
   hipError_t error = ::hipMemAdvise(ptr, size, advice, device);