From 052966c94b4e4a5494c5b977a5e073995d6a7d0d Mon Sep 17 00:00:00 2001 From: Benjamin Welton Date: Wed, 6 Sep 2023 22:06:06 -0700 Subject: [PATCH] Fix building of PC Sampling Experiment (#22) * Fix pc_sample building * source formatting (clang-format v11) (#43) Co-authored-by: jrmadsen * Update samples/pc_sampling/CMakeLists.txt * Allow static_asserts from hsa/types.hpp to be disabled via build flags (#24) * Fix pc_sample building * Fix up hsa type checks * Fix pc_sample building * source formatting (clang-format v11) * Revert check, now allow checks to be disabled by compiler defines. * Update samples/pc_sampling/common.h --------- Co-authored-by: bwelton Co-authored-by: Jonathan R. Madsen * Update single-user-host-trap-retries-service-instantiation.cpp - include vector --------- Co-authored-by: Jonathan R. Madsen Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: bwelton [ROCm/rocprofiler-sdk commit: 28272b3e5f937c599035628157555895e7d80f22] --- .../cmake/rocprofiler_build_settings.cmake | 5 ++++ .../cmake/rocprofiler_options.cmake | 2 ++ .../samples/pc_sampling/common.h | 10 ++++---- ...ost-trap-retries-service-instantiation.cpp | 23 ++++++++----------- .../pc_sampling/single-user-host-trap.cpp | 2 -- .../source/lib/rocprofiler/hsa/types.hpp | 12 ++++++---- 6 files changed, 28 insertions(+), 26 deletions(-) diff --git a/projects/rocprofiler-sdk/cmake/rocprofiler_build_settings.cmake b/projects/rocprofiler-sdk/cmake/rocprofiler_build_settings.cmake index 5791852202..60ea14bfbd 100644 --- a/projects/rocprofiler-sdk/cmake/rocprofiler_build_settings.cmake +++ b/projects/rocprofiler-sdk/cmake/rocprofiler_build_settings.cmake @@ -172,6 +172,11 @@ if(ROCPROFILER_BUILD_STATIC_LIBSTDCXX) INTERFACE rocprofiler::rocprofiler-static-libstdcxx) endif() +if(ROCPROFILER_UNSAFE_NO_VERSION_CHECK) + rocprofiler_target_compile_definitions(rocprofiler-build-flags + INTERFACE ROCPROFILER_UNSAFE_NO_VERSION_CHECK) +endif() + # ----------------------------------------------------------------------------------------# # user customization # diff --git a/projects/rocprofiler-sdk/cmake/rocprofiler_options.cmake b/projects/rocprofiler-sdk/cmake/rocprofiler_options.cmake index e55f2e3c75..6fd42db1d5 100644 --- a/projects/rocprofiler-sdk/cmake/rocprofiler_options.cmake +++ b/projects/rocprofiler-sdk/cmake/rocprofiler_options.cmake @@ -74,6 +74,8 @@ rocprofiler_add_option(ROCPROFILER_BUILD_STATIC_LIBSTDCXX "Build with -static-libstdc++ if possible" OFF ADVANCED) rocprofiler_add_option(ROCPROFILER_BUILD_STACK_PROTECTOR "Build with -fstack-protector" ON ADVANCED) +rocprofiler_add_option(ROCPROFILER_UNSAFE_NO_VERSION_CHECK + "Disable HSA version checking (for development only)" OFF ADVANCED) # In the future, we will do this even with clang-tidy enabled if(ROCPROFILER_BUILD_CI AND NOT ROCPROFILER_BUILD_WERROR) diff --git a/projects/rocprofiler-sdk/samples/pc_sampling/common.h b/projects/rocprofiler-sdk/samples/pc_sampling/common.h index 054a9b2041..fa49286a4a 100644 --- a/projects/rocprofiler-sdk/samples/pc_sampling/common.h +++ b/projects/rocprofiler-sdk/samples/pc_sampling/common.h @@ -16,8 +16,8 @@ const std::string_view MI200_NAME = "gfx90a"; #define ROCPROFILER_CALL(result, msg) \ { \ - rocprofiler_status_t status = result; \ - if(status != ROCPROFILER_STATUS_SUCCESS) \ + rocprofiler_status_t CHECKSTATUS = result; \ + if(CHECKSTATUS != ROCPROFILER_STATUS_SUCCESS) \ { \ puts(#result " failed"); \ } \ @@ -28,8 +28,8 @@ const std::string_view MI200_NAME = "gfx90a"; // after previous initialization. #define ROCPROFILER_CALL_FAILS(result, msg) \ { \ - rocprofiler_status_t status = result; \ - if(status == ROCPROFILER_STATUS_SUCCESS) \ + rocprofiler_status_t CHECKSTATUS = result; \ + if(CHECKSTATUS == ROCPROFILER_STATUS_SUCCESS) \ { \ puts(#result " succeeded"); \ } \ @@ -80,7 +80,7 @@ find_first_gpu_agent() ROCPROFILER_CALL(rocprofiler_query_available_agents(&find_first_gpu_agent_impl, sizeof(rocprofiler_agent_t), static_cast(&gpu_agent)), - "Failed to find GPU agents"); + "Could not query GPU agents"); return gpu_agent; } diff --git a/projects/rocprofiler-sdk/samples/pc_sampling/single-user-host-trap-retries-service-instantiation.cpp b/projects/rocprofiler-sdk/samples/pc_sampling/single-user-host-trap-retries-service-instantiation.cpp index 320d07ebaa..4cb534b1f9 100644 --- a/projects/rocprofiler-sdk/samples/pc_sampling/single-user-host-trap-retries-service-instantiation.cpp +++ b/projects/rocprofiler-sdk/samples/pc_sampling/single-user-host-trap-retries-service-instantiation.cpp @@ -10,6 +10,7 @@ #include "common.h" #include +#include #define HOST_TRAP_INTERVAL 1000 @@ -53,11 +54,11 @@ second_user() // After failure, the second user queries available configuration and observes the one chosen by // the first user. - rocprofiler_pc_sampling_configuration_t* configs; - size_t config_count; - ROCPROFILER_CALL( - rocprofiler_query_pc_sampling_agent_configurations(gpu_agent, configs, &config_count), - "The second user cannot query available configurations"); + size_t config_count = 10; + std::vector configs(config_count); + ROCPROFILER_CALL(rocprofiler_query_pc_sampling_agent_configurations( + gpu_agent, configs.data(), &config_count), + "The second user cannot query available configurations"); // Only one configuration should be listed, and its parameters should match the parameters set // by the first user. Vladimir: Is it ok to use assertions? In the release mode, they might be @@ -71,9 +72,6 @@ second_user() assert(first_user_config.min_interval == host_trap_interval && first_user_config.min_interval == first_user_config.max_interval); - // Vladimir: Do we need to explicitly free queried configurations? - free(configs); - // Reuse the same configuration set by the first user. // The second user is satisfied with the configuration chosen by the first user, so it // starts PC sampling. @@ -106,8 +104,6 @@ second_user() int main(int /*argc*/, char** /*argv*/) { - rocprofiler_status_t status; - // creating a context rocprofiler_context_id_t context_id; ROCPROFILER_CALL(rocprofiler_create_context(&context_id), "Cannot create context\n"); @@ -127,11 +123,10 @@ main(int /*argc*/, char** /*argv*/) "Cannot create pc sampling buffer"); // PC sampling service configuration - rocprofiler_pc_sampling_method_t host_trap_sampling_method = - ROCPROFILER_PC_SAMPLING_METHOD_HOST_TRAP; - rocprofiler_pc_sampling_unit_t host_trap_sampling_unit_time = ROCPROFILER_PC_SAMPLING_UNIT_TIME; + host_trap_sampling_method = ROCPROFILER_PC_SAMPLING_METHOD_HOST_TRAP; + host_trap_sampling_unit_time = ROCPROFILER_PC_SAMPLING_UNIT_TIME; // Vladimir: What units are we using for time? ms, micro secs, ns? - uint64_t host_trap_interval = HOST_TRAP_INTERVAL; + host_trap_interval = HOST_TRAP_INTERVAL; // Instantiating the first PC sampling service succeeds. ROCPROFILER_CALL(rocprofiler_configure_pc_sampling_service(context_id, gpu_agent, diff --git a/projects/rocprofiler-sdk/samples/pc_sampling/single-user-host-trap.cpp b/projects/rocprofiler-sdk/samples/pc_sampling/single-user-host-trap.cpp index cc1fa5a7d7..8eb3de78bc 100644 --- a/projects/rocprofiler-sdk/samples/pc_sampling/single-user-host-trap.cpp +++ b/projects/rocprofiler-sdk/samples/pc_sampling/single-user-host-trap.cpp @@ -7,8 +7,6 @@ int main(int /*argc*/, char** /*argv*/) { - rocprofiler_status_t status; - // creating a context rocprofiler_context_id_t context_id; ROCPROFILER_CALL(rocprofiler_create_context(&context_id), "Cannot create context\n"); diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler/hsa/types.hpp b/projects/rocprofiler-sdk/source/lib/rocprofiler/hsa/types.hpp index e8d4b8d380..7196f51880 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler/hsa/types.hpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler/hsa/types.hpp @@ -22,8 +22,9 @@ #include "rocprofiler/hsa.h" -#if defined(ROCPROFILER_CI) && ROCPROFILER_CI > 0 -# if HSA_API_TABLE_MAJOR_VERSION <= 0x01 +#ifndef ROCPROFILER_UNSAFE_NO_VERSION_CHECK +# if defined(ROCPROFILER_CI) && ROCPROFILER_CI > 0 +# if HSA_API_TABLE_MAJOR_VERSION <= 0x01 static_assert(HSA_CORE_API_TABLE_MAJOR_VERSION == 0x01, "Change in the major version of HSA core API table"); static_assert(HSA_AMD_EXT_API_TABLE_MAJOR_VERSION == 0x01, @@ -52,7 +53,8 @@ static_assert(sizeof(FinalizerExtTable) == 64, "HSA finalizer API table size cha static_assert(sizeof(ImageExtTable) == 120, "HSA image-extended API table size changed"); static_assert(sizeof(AmdExtTable) == 552, "HSA amd-extended API table size changed"); static_assert(sizeof(CoreApiTable) == 1016, "HSA core API table size changed"); -# else -# error "HSA_API_TABLE_MAJOR_VERSION not supported" +# else +# error "HSA_API_TABLE_MAJOR_VERSION not supported" +# endif # endif -#endif +#endif \ No newline at end of file