From d820cf3018d2a60e7ccc70590c437997b1dbd6a7 Mon Sep 17 00:00:00 2001 From: "Jonathan R. Madsen" Date: Wed, 6 Mar 2024 02:17:40 -0600 Subject: [PATCH] Update `rocprofiler_query_available_agents(...)` (#596) * Agent info version * Complete implementation - revert "rocprofiler_iterate_agents" to "rocprofiler_query_available_agents" * Misc tweaks - update rocprofiler_query_available_agents impl * Update include/rocprofiler-sdk/agent.h - Fix undocumented param for rocprofiler_query_available_agents [ROCm/rocprofiler-sdk commit: 1d33d4cf788ce5cf9a3c1fca5762be2b28c37c7b] --- .../source/include/rocprofiler-sdk/agent.h | 40 ++++++++++++++----- .../source/lib/rocprofiler-sdk-tool/tool.cpp | 18 ++++++--- .../source/lib/rocprofiler-sdk/agent.cpp | 35 +++++++++++----- .../lib/rocprofiler-sdk/tests/agent.cpp | 36 +++++++++++------ .../rocprofiler-sdk/tests/tools/json-tool.cpp | 22 ++++++---- 5 files changed, 106 insertions(+), 45 deletions(-) diff --git a/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/agent.h b/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/agent.h index 67d5d22536..47f93b7baa 100644 --- a/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/agent.h +++ b/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/agent.h @@ -38,6 +38,17 @@ ROCPROFILER_EXTERN_C_INIT * @{ */ +/** + * @brief Enumeration ID for version of the rocprofiler_agent_v*_t struct in rocprofiler_i + * + */ +typedef enum rocprofiler_agent_version_t +{ + ROCPROFILER_AGENT_INFO_VERSION_NONE = 0, + ROCPROFILER_AGENT_INFO_VERSION_0 = 1, + ROCPROFILER_AGENT_INFO_VERSION_LAST, +} rocprofiler_agent_version_t; + /** * @brief Cache information for an agent. */ @@ -89,7 +100,7 @@ typedef struct rocprofiler_agent_mem_bank_t /** * @brief Agent. */ -typedef struct rocprofiler_agent_t +typedef struct rocprofiler_agent_v0_t { uint64_t size; ///< set to sizeof(rocprofiler_agent_t) by rocprofiler. This can be used for ///< versioning and compatibility handling @@ -179,33 +190,44 @@ typedef struct rocprofiler_agent_t uint32_t node_id; ///< Node sequence number. This will be equivalent to the HSA-runtime ///< HSA_AMD_AGENT_INFO_DRIVER_NODE_ID property uint32_t reserved0; ///< reserved padding -} rocprofiler_agent_t; +} rocprofiler_agent_v0_t; + +typedef rocprofiler_agent_v0_t rocprofiler_agent_t; /** - * @brief Callback function type for querying the available agents + * @brief Callback function type for querying the available agents. * + * If callback is invoked, returns the ::rocprofiler_status_t value returned from callback + * + * @param [in] version Enum specifying the version of agent info * @param [in] agents Array of pointers to agents * @param [in] num_agents Number of agents in array * @param [in] user_data Data pointer passback * @return ::rocprofiler_status_t + * @retval ::ROCPROFILER_STATUS_ERROR_INCOMPATIBLE_ABI size of the agent struct in application is + * larger than the agent struct for rocprofiler-sdk + * @retval ::ROCPROFILER_STATUS_ERROR_INVALID_ARGUMENT Invalid ::rocprofiler_agent_version_t value */ -typedef rocprofiler_status_t (*rocprofiler_available_agents_cb_t)( - const rocprofiler_agent_t** agents, +typedef rocprofiler_status_t (*rocprofiler_query_available_agents_cb_t)( + rocprofiler_agent_version_t version, + const void** agents, size_t num_agents, void* user_data); /** * @brief Receive synchronous callback with an array of available agents at moment of invocation * + * @param [in] version Enum value specifying the struct type of the agent info * @param [in] callback Callback function accepting list of agents * @param [in] agent_size Should be set to sizeof(rocprofiler_agent_t) * @param [in] user_data Data pointer provided to callback * @return ::rocprofiler_status_t */ -rocprofiler_status_t ROCPROFILER_API -rocprofiler_query_available_agents(rocprofiler_available_agents_cb_t callback, - size_t agent_size, - void* user_data) ROCPROFILER_NONNULL(1); +rocprofiler_status_t +rocprofiler_query_available_agents(rocprofiler_agent_version_t version, + rocprofiler_query_available_agents_cb_t callback, + size_t agent_size, + void* user_data) ROCPROFILER_API ROCPROFILER_NONNULL(2); /** @} */ diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/tool.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/tool.cpp index a3d4a75b79..43ea6cf7b9 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/tool.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/tool.cpp @@ -804,15 +804,19 @@ get_agent_profile(const rocprofiler_agent_t* agent) } rocprofiler_status_t -list_metrics_iterate_agents(const rocprofiler_agent_t** agents, size_t num_agents, void*) +list_metrics_iterate_agents(rocprofiler_agent_version_t, + const void** agents, + size_t num_agents, + void*) { for(size_t idx = 0; idx < num_agents; idx++) { - auto counters_v = counter_vec_t{}; - uint32_t node_id = agents[idx]->node_id; + const auto* agent = static_cast(agents[idx]); + auto counters_v = counter_vec_t{}; + uint32_t node_id = agent->node_id; ROCPROFILER_CALL( rocprofiler_iterate_agent_supported_counters( - agents[idx]->id, + agent->id, [](rocprofiler_agent_id_t, rocprofiler_counter_id_t* counters, size_t num_counters, @@ -1149,8 +1153,10 @@ api_registration_callback(rocprofiler_intercept_table_t, uint64_t, void*) { - ROCPROFILER_CALL(rocprofiler_query_available_agents( - list_metrics_iterate_agents, sizeof(rocprofiler_agent_t), nullptr), + ROCPROFILER_CALL(rocprofiler_query_available_agents(ROCPROFILER_AGENT_INFO_VERSION_0, + list_metrics_iterate_agents, + sizeof(rocprofiler_agent_t), + nullptr), "Iterate rocporfiler agents") } diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/agent.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/agent.cpp index e8fb7b0f78..5ce4468991 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/agent.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/agent.cpp @@ -877,18 +877,35 @@ get_agent_available_properties() extern "C" { rocprofiler_status_t -rocprofiler_query_available_agents(rocprofiler_available_agents_cb_t callback, - size_t agent_size, - void* user_data) +rocprofiler_query_available_agents(rocprofiler_agent_version_t version, + rocprofiler_query_available_agents_cb_t callback, + size_t agent_size, + void* user_data) { - if(agent_size > sizeof(rocprofiler_agent_t)) + // only support version 0 for now + if(version != ROCPROFILER_AGENT_INFO_VERSION_0) + return ROCPROFILER_STATUS_ERROR_INVALID_ARGUMENT; + + // this will need to be updated for new versions + if(version == ROCPROFILER_AGENT_INFO_VERSION_0) { - LOG(ERROR) << "rocprofiler_agent_t used by caller is ABI-incompatible with " - "rocprofiler_agent_t in rocprofiler"; - return ROCPROFILER_STATUS_ERROR_INCOMPATIBLE_ABI; + if(agent_size > sizeof(rocprofiler_agent_v0_t)) + { + LOG(ERROR) << "size of rocprofiler agent struct used by caller is ABI-incompatible " + "with rocprofiler_agent_v0_t in rocprofiler"; + return ROCPROFILER_STATUS_ERROR_INCOMPATIBLE_ABI; + } + } + else + { + LOG(FATAL) << "rocprofiler-sdk does not support given agent info version"; } - auto&& pointers = rocprofiler::agent::get_agents(); - return callback(pointers.data(), pointers.size(), user_data); + auto&& pointers = rocprofiler::agent::get_agents(); + auto v_pointers = std::vector{}; + v_pointers.reserve(pointers.size()); + for(const auto& itr : pointers) + v_pointers.emplace_back(itr); + return callback(version, v_pointers.data(), pointers.size(), user_data); } } diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/agent.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/agent.cpp index f4cab77147..27935270fb 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/agent.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/agent.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include TEST(rocprofiler_lib, agent_abi) @@ -132,22 +133,31 @@ TEST(rocprofiler_lib, agent) "#####\n\"; cat ${i}; echo \"\"; done'"); EXPECT_EQ(sys_ret_virt, 0); - auto agents = std::vector{}; - rocprofiler_available_agents_cb_t iterate_cb = - [](const rocprofiler_agent_t** agents_arr, size_t num_agents, void* user_data) { - auto* agents_v = static_cast*>(user_data); - // EXPECT_EQ(num_agents, hsa_agents_v.size()); - for(size_t i = 0; i < num_agents; ++i) - { - const auto* agent = agents_arr[i]; - agents_v->emplace_back(agent); - } - return ROCPROFILER_STATUS_SUCCESS; - }; + static_assert(std::is_same::value, + "update test to support new agent struct version"); + + auto agents = std::vector{}; + rocprofiler_query_available_agents_cb_t iterate_cb = [](rocprofiler_agent_version_t agents_ver, + const void** agents_arr, + size_t num_agents, + void* user_data) { + EXPECT_EQ(agents_ver, ROCPROFILER_AGENT_INFO_VERSION_0); + if(agents_ver != ROCPROFILER_AGENT_INFO_VERSION_0) return ROCPROFILER_STATUS_ERROR; + + auto* agents_v = static_cast*>(user_data); + // EXPECT_EQ(num_agents, hsa_agents_v.size()); + for(size_t i = 0; i < num_agents; ++i) + { + const auto* agent = static_cast(agents_arr[i]); + agents_v->emplace_back(agent); + } + return ROCPROFILER_STATUS_SUCCESS; + }; std::cout << "# querying available agents...\n" << std::flush; auto status = - rocprofiler_query_available_agents(iterate_cb, + rocprofiler_query_available_agents(ROCPROFILER_AGENT_INFO_VERSION_0, + iterate_cb, sizeof(rocprofiler_agent_t), const_cast(static_cast(&agents))); diff --git a/projects/rocprofiler-sdk/tests/tools/json-tool.cpp b/projects/rocprofiler-sdk/tests/tools/json-tool.cpp index 82d41948ba..21be82fbed 100644 --- a/projects/rocprofiler-sdk/tests/tools/json-tool.cpp +++ b/projects/rocprofiler-sdk/tests/tools/json-tool.cpp @@ -36,6 +36,7 @@ #include "common/perfetto.hpp" #include "common/serialization.hpp" +#include #include #include #include @@ -903,16 +904,21 @@ tool_init(rocprofiler_client_finalize_t fini_func, void* tool_data) assert(tool_data != nullptr); - rocprofiler_available_agents_cb_t iterate_cb = - [](const rocprofiler_agent_t** agents_arr, size_t num_agents, void* user_data) { - auto* agents_v = static_cast*>(user_data); - for(size_t i = 0; i < num_agents; ++i) - agents_v->emplace_back(*agents_arr[i]); - return ROCPROFILER_STATUS_SUCCESS; - }; + rocprofiler_query_available_agents_cb_t iterate_cb = [](rocprofiler_agent_version_t agents_ver, + const void** agents_arr, + size_t num_agents, + void* user_data) { + if(agents_ver != ROCPROFILER_AGENT_INFO_VERSION_0) + throw std::runtime_error{"unexpected rocprofiler agent version"}; + auto* agents_v = static_cast*>(user_data); + for(size_t i = 0; i < num_agents; ++i) + agents_v->emplace_back(*static_cast(agents_arr[i])); + return ROCPROFILER_STATUS_SUCCESS; + }; ROCPROFILER_CALL( - rocprofiler_query_available_agents(iterate_cb, + rocprofiler_query_available_agents(ROCPROFILER_AGENT_INFO_VERSION_0, + iterate_cb, sizeof(rocprofiler_agent_t), const_cast(static_cast(&agents))), "query available agents");