[SDK] Fix null handles (#474)

* Fix null handle

- use .handle=0, not .handle=numeric_limits<>::max()

* Update lib.common.hasher

* Fix ROCPROFILER_CONTEXT_NONE

* Use context operator==

* Update CHANGELOG

* Updated null handle for scratch memory and changed allocation test so that free ops account for null agent

---------

Co-authored-by: Ian Trowbridge <Ian.Trowbridge@amd.com>

source/include/rocprofiler-sdk/context.h

@@ -38,7 +38,7 @@ ROCPROFILER_EXTERN_C_INIT
 /**
  * The NULL Context handle.
  */
-#define ROCPROFILER_CONTEXT_NONE ROCPROFILER_HANDLE_LITERAL(rocprofiler_context_id_t, UINT64_MAX)
+#define ROCPROFILER_CONTEXT_NONE ROCPROFILER_HANDLE_LITERAL(rocprofiler_context_id_t, 0UL)
 
 /**
  * @brief Create context.

source/lib/common/hasher.hpp

@@ -28,7 +28,9 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <optional>
 #include <string>
+#include <string_view>
 #include <type_traits>
 
 namespace rocprofiler
@@ -53,17 +55,20 @@ struct fnv1a_hasher
 
     // Hashes a numeric value.
     template <typename Tp, typename std::enable_if_t<std::is_arithmetic<Tp>::value, bool> = true>
-    void update(Tp data);
+    fnv1a_hasher& update(Tp data);
+
+    template <typename Tp>
+    fnv1a_hasher& update(const std::optional<Tp>& data);
 
     // Using the loop instead of "update(str, strlen(str))" to avoid looping twice
-    void update(const char* str);
+    fnv1a_hasher& update(const char* str);
 
     // Hashes a byte array.
-    void update(const char* data, size_t size);
+    fnv1a_hasher& update(const char* data, size_t size);
 
-    void update(std::string_view s) { update(s.data(), s.size()); }
+    fnv1a_hasher& update(std::string_view s) { return update(s.data(), s.size()); }
 
-    void update(const std::string& s) { update(s.data(), s.size()); }
+    fnv1a_hasher& update(const std::string& s) { return update(s.data(), s.size()); }
 
     // Usage: uint64_t hashed_value = Hash::combine(33, false, "ABC", 458L, 3u, 'x');
     template <typename Tp, typename... TailT>
@@ -72,7 +77,7 @@ struct fnv1a_hasher
     // fnv1a_hasher.update_all(33, false, "ABC")` is shorthand for calling fnv1a_hasher.update(...)
     // for each value in the same order
     template <typename Tp, typename... TailT>
-    void update_all(Tp&& arg, TailT&&... args);
+    fnv1a_hasher& update_all(Tp&& arg, TailT&&... args);
 
     uint64_t digest() const { return m_result; }
 
@@ -84,13 +89,20 @@ private:
 };
 
 template <typename Tp, typename std::enable_if_t<std::is_arithmetic<Tp>::value, bool>>
-void
+fnv1a_hasher&
 fnv1a_hasher::update(Tp data)
 {
-    update(reinterpret_cast<const char*>(&data), sizeof(data));
+    return update(reinterpret_cast<const char*>(&data), sizeof(data));
 }
 
-inline void
+template <typename Tp>
+fnv1a_hasher&
+fnv1a_hasher::update(const std::optional<Tp>& data)
+{
+    return (data) ? update(*data) : *this;
+}
+
+inline fnv1a_hasher&
 fnv1a_hasher::update(const char* str)
 {
     constexpr auto max_n = 4096;
@@ -101,9 +113,10 @@ fnv1a_hasher::update(const char* str)
         update(*p);
         if(++n >= max_n) break;  // prevent infinite loop
     }
+    return *this;
 }
 
-inline void
+inline fnv1a_hasher&
 fnv1a_hasher::update(const char* data, size_t size)
 {
     for(size_t i = 0; i < size; ++i)
@@ -113,23 +126,23 @@ fnv1a_hasher::update(const char* data, size_t size)
         // signed integers.
         m_result *= kFnv1a64Prime;
     }
+    return *this;
 }
 
 template <typename Tp, typename... TailT>
 uint64_t
 fnv1a_hasher::combine(Tp&& arg, TailT&&... args)
 {
-    auto _hasher = fnv1a_hasher{};
-    _hasher.update_all(std::forward<Tp>(arg), std::forward<TailT>(args)...);
-    return _hasher.digest();
+    return fnv1a_hasher{}.update_all(std::forward<Tp>(arg), std::forward<TailT>(args)...).digest();
 }
 
 template <typename Tp, typename... TailT>
-void
+fnv1a_hasher&
 fnv1a_hasher::update_all(Tp&& arg, TailT&&... args)
 {
     update(arg);
     if constexpr(sizeof...(TailT) > 0) update_all(std::forward<TailT>(args)...);
+    return *this;
 }
 }  // namespace common
 }  // namespace rocprofiler

source/lib/output/generateOTF2.cpp

@@ -491,9 +491,8 @@ write_otf2(const output_config&                                          cfg,
         {
             // Free functions do not track agent information. Below handles case where
             // null rocprof agent id is passed to generate OTF2
-            constexpr auto null_rocp_agent_id =
-                rocprofiler_agent_id_t{.handle = std::numeric_limits<uint64_t>::max()};
-            const rocprofiler_agent_t* _agent = nullptr;
+            constexpr auto             null_rocp_agent_id = rocprofiler_agent_id_t{.handle = 0};
+            const rocprofiler_agent_t* _agent             = nullptr;
             if(agent != null_rocp_agent_id)
             {
                 _agent = _get_agent(agent);
@@ -896,9 +895,8 @@ write_otf2(const output_config&                                          cfg,
             auto _hash = get_hash_id(evt.name);
             // Using max numeric limits results in an out-of-bound runtime error for OTF2
             // and perfetto for agent ids. Setting handle to 0 for free functions.
-            constexpr auto null_rocp_agent_id =
-                rocprofiler_agent_id_t{.handle = std::numeric_limits<uint64_t>::max()};
-            auto handle = agent.handle;
+            constexpr auto null_rocp_agent_id = rocprofiler_agent_id_t{.handle = 0};
+            auto           handle             = agent.handle;
             if(agent == null_rocp_agent_id) handle = 0;
 
             add_write_string(_hash, evt.name);

source/lib/output/generatePerfetto.cpp

@@ -837,8 +837,7 @@ write_perfetto(
         }
 
         // memory allocation counter track
-        constexpr auto null_rocp_agent_id =
-            rocprofiler_agent_id_t{.handle = std::numeric_limits<uint64_t>::max()};
+        constexpr auto null_rocp_agent_id = rocprofiler_agent_id_t{.handle = 0};
         struct free_memory_information
         {
             rocprofiler_timestamp_t start_timestamp = 0;
@@ -855,9 +854,8 @@ write_perfetto(
 
         struct agent_and_size
         {
-            rocprofiler_agent_id_t agent_id =
-                rocprofiler_agent_id_t{.handle = std::numeric_limits<uint64_t>::max()};
-            uint64_t size = {0};
+            rocprofiler_agent_id_t agent_id = rocprofiler_agent_id_t{.handle = 0};
+            uint64_t               size     = {0};
         };
 
         auto mem_alloc_endpoints =

source/lib/rocprofiler-sdk/buffer_tracing.cpp

@@ -159,8 +159,7 @@ rocprofiler_configure_buffer_tracing_service(rocprofiler_context_id_t
 
     if(buffer_id.handle == 0) return ROCPROFILER_STATUS_ERROR_BUFFER_NOT_FOUND;
 
-    constexpr auto invalid_buffer_id =
-        rocprofiler_buffer_id_t{std::numeric_limits<uint64_t>::max()};
+    constexpr auto invalid_buffer_id = rocprofiler_buffer_id_t{0};
 
     if(!ctx->buffered_tracer)
     {
@@ -168,7 +167,7 @@ rocprofiler_configure_buffer_tracing_service(rocprofiler_context_id_t
         ctx->buffered_tracer->buffer_data.fill(invalid_buffer_id);
     }
 
-    if(ctx->buffered_tracer->buffer_data.at(kind).handle != invalid_buffer_id.handle)
+    if(ctx->buffered_tracer->buffer_data.at(kind) != invalid_buffer_id)
         return ROCPROFILER_STATUS_ERROR_SERVICE_ALREADY_CONFIGURED;
 
     RETURN_STATUS_ON_FAIL(rocprofiler::context::add_domain(ctx->buffered_tracer->domains, kind));

source/lib/rocprofiler-sdk/context.cpp

@@ -20,14 +20,15 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 // SOFTWARE.
 
-#include <rocprofiler-sdk/context.h>
-#include <rocprofiler-sdk/fwd.h>
-
 #include "lib/rocprofiler-sdk/context/context.hpp"
 #include "lib/rocprofiler-sdk/context/domain.hpp"
 #include "lib/rocprofiler-sdk/hsa/hsa.hpp"
 #include "lib/rocprofiler-sdk/registration.hpp"
 
+#include <rocprofiler-sdk/context.h>
+#include <rocprofiler-sdk/fwd.h>
+#include <rocprofiler-sdk/cxx/operators.hpp>
+
 #include <atomic>
 #include <vector>
 
@@ -60,7 +61,7 @@ rocprofiler_create_context(rocprofiler_context_id_t* context_id)
 rocprofiler_status_t
 rocprofiler_start_context(rocprofiler_context_id_t context_id)
 {
-    if(context_id.handle == rocprofiler_context_none.handle ||
+    if(context_id == rocprofiler_context_none ||
        !rocprofiler::context::get_registered_context(context_id))
         return ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND;
 
@@ -74,7 +75,7 @@ rocprofiler_start_context(rocprofiler_context_id_t context_id)
 rocprofiler_status_t
 rocprofiler_stop_context(rocprofiler_context_id_t context_id)
 {
-    if(context_id.handle == rocprofiler_context_none.handle ||
+    if(context_id == rocprofiler_context_none ||
        !rocprofiler::context::get_registered_context(context_id))
         return ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND;
 
@@ -91,7 +92,7 @@ rocprofiler_context_is_active(rocprofiler_context_id_t context_id, int* status)
     *status = 0;
 
     // return context not found if not registered
-    if(context_id.handle == rocprofiler_context_none.handle ||
+    if(context_id == rocprofiler_context_none ||
        !rocprofiler::context::get_registered_context(context_id))
         return ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND;
 
@@ -114,7 +115,7 @@ rocprofiler_context_is_valid(rocprofiler_context_id_t context_id, int* status)
 {
     *status = 0;
 
-    if(context_id.handle == rocprofiler_context_none.handle ||
+    if(context_id == rocprofiler_context_none ||
        !rocprofiler::context::get_registered_context(context_id))
         return ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND;
 

source/lib/rocprofiler-sdk/hsa/async_copy.cpp

@@ -142,8 +142,7 @@ context_filter(const context::context* ctx)
     return (has_buffered || has_callback);
 }
 
-constexpr auto null_rocp_agent_id =
-    rocprofiler_agent_id_t{.handle = std::numeric_limits<uint64_t>::max()};
+constexpr auto null_rocp_agent_id = rocprofiler_agent_id_t{.handle = 0};
 
 struct async_copy_data
 {

source/lib/rocprofiler-sdk/hsa/memory_allocation.cpp

@@ -324,8 +324,7 @@ get_next_dispatch()
     return _v;
 }
 
-constexpr auto null_rocp_agent_id =
-    rocprofiler_agent_id_t{.handle = std::numeric_limits<uint64_t>::max()};
+constexpr auto null_rocp_agent_id = rocprofiler_agent_id_t{.handle = 0};
 
 struct memory_allocation_data
 {

source/lib/rocprofiler-sdk/hsa/queue_controller.cpp

@@ -83,9 +83,9 @@ destroy_queue(hsa_queue_t* hsa_queue)
 }
 
 constexpr rocprofiler_agent_t default_agent =
-    rocprofiler_agent_t{.size = sizeof(rocprofiler_agent_t),
-                        .id   = rocprofiler_agent_id_t{std::numeric_limits<uint64_t>::max()},
-                        .type = ROCPROFILER_AGENT_TYPE_NONE,
+    rocprofiler_agent_t{.size                       = sizeof(rocprofiler_agent_t),
+                        .id                         = rocprofiler_agent_id_t{.handle = 0},
+                        .type                       = ROCPROFILER_AGENT_TYPE_NONE,
                         .cpu_cores_count            = 0,
                         .simd_count                 = 0,
                         .mem_banks_count            = 0,
@@ -129,8 +129,8 @@ constexpr rocprofiler_agent_t default_agent =
                         .local_mem_size             = 0,
                         .hive_id                    = 0,
                         .gpu_id                     = 0,
-                        .workgroup_max_dim          = {0, 0, 0},
-                        .grid_max_dim               = {0, 0, 0},
+                        .workgroup_max_dim          = {.x = 0, .y = 0, .z = 0},
+                        .grid_max_dim               = {.x = 0, .y = 0, .z = 0},
                         .mem_banks                  = nullptr,
                         .caches                     = nullptr,
                         .io_links                   = nullptr,

source/lib/rocprofiler-sdk/hsa/scratch_memory.cpp

@@ -462,7 +462,7 @@ impl(Args... args)
 
     [[maybe_unused]] const auto get_agent_id =
         [](const hsa_queue_t* hsa_queue) -> rocprofiler_agent_id_t {
-        rocprofiler_agent_id_t _agent_id{static_cast<uint64_t>(-1)};
+        rocprofiler_agent_id_t _agent_id{.handle = 0};
         bool                   found_agent{false};
 
         rocprofiler::hsa::get_queue_controller()->iterate_queues(