diff --git a/projects/rocprofiler-sdk/CHANGELOG.md b/projects/rocprofiler-sdk/CHANGELOG.md index b4f06deace..667877518d 100644 --- a/projects/rocprofiler-sdk/CHANGELOG.md +++ b/projects/rocprofiler-sdk/CHANGELOG.md @@ -204,6 +204,8 @@ Full documentation for ROCprofiler-SDK is available at [rocm.docs.amd.com/projec - rocprofv3 avail tool renamed from rocprofv3_avail to rocprofv3-avail tool - rocprofv3 avail tool has support for command line arguments. - rocprofv3 tool now allows for Thread Trace + PC Sampling on the same agent +- fixed inconsistency for what is a "null" handle in `rocprofiler_*_id_t` structs. + - correct answer is `.handle = 0` but some definitions used `UINT64_MAX` ### Resolved issues diff --git a/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/context.h b/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/context.h index af3a22d2b3..4d4a321ae6 100644 --- a/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/context.h +++ b/projects/rocprofiler-sdk/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. diff --git a/projects/rocprofiler-sdk/source/lib/common/hasher.hpp b/projects/rocprofiler-sdk/source/lib/common/hasher.hpp index 22b0b2326d..00da3c496f 100644 --- a/projects/rocprofiler-sdk/source/lib/common/hasher.hpp +++ b/projects/rocprofiler-sdk/source/lib/common/hasher.hpp @@ -28,7 +28,9 @@ #include #include +#include #include +#include #include namespace rocprofiler @@ -53,17 +55,20 @@ struct fnv1a_hasher // Hashes a numeric value. template ::value, bool> = true> - void update(Tp data); + fnv1a_hasher& update(Tp data); + + template + fnv1a_hasher& update(const std::optional& 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 @@ -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 - 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 ::value, bool>> -void +fnv1a_hasher& fnv1a_hasher::update(Tp data) { - update(reinterpret_cast(&data), sizeof(data)); + return update(reinterpret_cast(&data), sizeof(data)); } -inline void +template +fnv1a_hasher& +fnv1a_hasher::update(const std::optional& 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 uint64_t fnv1a_hasher::combine(Tp&& arg, TailT&&... args) { - auto _hasher = fnv1a_hasher{}; - _hasher.update_all(std::forward(arg), std::forward(args)...); - return _hasher.digest(); + return fnv1a_hasher{}.update_all(std::forward(arg), std::forward(args)...).digest(); } template -void +fnv1a_hasher& fnv1a_hasher::update_all(Tp&& arg, TailT&&... args) { update(arg); if constexpr(sizeof...(TailT) > 0) update_all(std::forward(args)...); + return *this; } } // namespace common } // namespace rocprofiler diff --git a/projects/rocprofiler-sdk/source/lib/output/generateOTF2.cpp b/projects/rocprofiler-sdk/source/lib/output/generateOTF2.cpp index 7c31f8b187..d95894fecb 100644 --- a/projects/rocprofiler-sdk/source/lib/output/generateOTF2.cpp +++ b/projects/rocprofiler-sdk/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::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::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); diff --git a/projects/rocprofiler-sdk/source/lib/output/generatePerfetto.cpp b/projects/rocprofiler-sdk/source/lib/output/generatePerfetto.cpp index 56e365819c..315fa7a642 100644 --- a/projects/rocprofiler-sdk/source/lib/output/generatePerfetto.cpp +++ b/projects/rocprofiler-sdk/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::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::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 = diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/buffer_tracing.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/buffer_tracing.cpp index a5104517f5..1e72623ab2 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/buffer_tracing.cpp +++ b/projects/rocprofiler-sdk/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::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)); diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context.cpp index e04e530581..8397eef5d0 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context.cpp +++ b/projects/rocprofiler-sdk/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 -#include - #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 +#include +#include + #include #include @@ -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; diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/async_copy.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/async_copy.cpp index 50e7da6bb9..ab81c2d958 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/async_copy.cpp +++ b/projects/rocprofiler-sdk/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::max()}; +constexpr auto null_rocp_agent_id = rocprofiler_agent_id_t{.handle = 0}; struct async_copy_data { diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/memory_allocation.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/memory_allocation.cpp index 16f0320899..4a51ae1a57 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/memory_allocation.cpp +++ b/projects/rocprofiler-sdk/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::max()}; +constexpr auto null_rocp_agent_id = rocprofiler_agent_id_t{.handle = 0}; struct memory_allocation_data { diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue_controller.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue_controller.cpp index 3099b06bec..557cfd1dd5 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue_controller.cpp +++ b/projects/rocprofiler-sdk/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::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, diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/scratch_memory.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/scratch_memory.cpp index 2eb3fd36e0..e7a5ba063c 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/scratch_memory.cpp +++ b/projects/rocprofiler-sdk/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(-1)}; + rocprofiler_agent_id_t _agent_id{.handle = 0}; bool found_agent{false}; rocprofiler::hsa::get_queue_controller()->iterate_queues( diff --git a/projects/rocprofiler-sdk/tests/rocprofv3/memory-allocation/validate.py b/projects/rocprofiler-sdk/tests/rocprofv3/memory-allocation/validate.py index 62a7474c79..f3cb03cb64 100755 --- a/projects/rocprofiler-sdk/tests/rocprofv3/memory-allocation/validate.py +++ b/projects/rocprofiler-sdk/tests/rocprofv3/memory-allocation/validate.py @@ -59,7 +59,18 @@ def test_memory_allocation(json_data): assert len(bf_op_names) == 5 - allocation_reported_agent_ids = set() + # Op values: + UNKNOWN_OP = 0 + HSA_MEMORY_ALLOCATE_OP = 1 + HSA_AMD_VMEM_HANDLE_CREATE_OP = 2 + HSA_MEMORY_FREE_OP = 3 + HSA_AMD_VMEM_HANDLE_RELEASE = 4 + + valid_agent_ids = set() + for row in data["agents"]: + if "id" in row and "handle" in row.id: + valid_agent_ids.add(row.id.handle) + # check buffering data for node in memory_allocation_data: assert "size" in node @@ -78,7 +89,14 @@ def test_memory_allocation(json_data): assert node.allocation_size >= 0 assert len(node.address) > 0 assert node.thread_id > 0 - assert node.agent_id.handle > 0 + + op_id = node.operation + assert op_id != UNKNOWN_OP + if op_id == HSA_MEMORY_ALLOCATE_OP or op_id == HSA_AMD_VMEM_HANDLE_CREATE_OP: + assert node.agent_id.handle in valid_agent_ids + else: + assert node.agent_id.handle == 0 # free ops record agent id as null + assert node.start_timestamp > 0 assert node.end_timestamp > 0 assert node.start_timestamp < node.end_timestamp @@ -89,8 +107,6 @@ def test_memory_allocation(json_data): in bf_op_names ) - allocation_reported_agent_ids.add(node["agent_id"]["handle"]) - def test_otf2_data(otf2_data, json_data): import rocprofiler_sdk.tests.rocprofv3 as rocprofv3