From bd447ab941b7ac655bf2acce7a8af2a1396e9d3b Mon Sep 17 00:00:00 2001 From: "Madsen, Jonathan" Date: Fri, 6 Dec 2024 12:35:29 -0600 Subject: [PATCH] Misc AFAR VII updates + clang-tidy-19 + bump version to 0.6.0 (#54) * Misc AFAR VII updates + clang-tidy-19 + bump version to 0.6.0 - move tests/rocprofv3/trace-period to tests/rocprofv3/collection-period - bump clang-tidy to v19 - fix misc clang-tidy errors * Update the collection period test - don't attach files on fail bc when test is disabled, it causes problems --------- Co-authored-by: Jonathan R. Madsen --- CMakeLists.txt | 3 ++ VERSION | 2 +- requirements.txt | 2 +- .../rocprofiler-sdk/cxx/serialization.hpp | 1 - .../common/container/record_header_buffer.cpp | 4 +- source/lib/common/container/small_vector.cpp | 2 +- source/lib/rocprofiler-sdk-tool/tool.cpp | 6 +-- source/lib/rocprofiler-sdk/agent.cpp | 48 +++++++++---------- .../rocprofiler-sdk/hsa/memory_allocation.cpp | 30 +++++------- tests/rocprofv3/CMakeLists.txt | 2 +- .../CMakeLists.txt | 29 ++++++----- .../conftest.py | 0 .../pytest.ini | 0 .../validate.py | 0 14 files changed, 65 insertions(+), 64 deletions(-) rename tests/rocprofv3/{trace-period => collection-period}/CMakeLists.txt (78%) rename tests/rocprofv3/{trace-period => collection-period}/conftest.py (100%) rename tests/rocprofv3/{trace-period => collection-period}/pytest.ini (100%) rename tests/rocprofv3/{trace-period => collection-period}/validate.py (100%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2af49b6bc5..a9dbe9efe0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -61,6 +61,9 @@ else() set(ROCPROFILER_GIT_REVISION "") endif() +# make sure that cmake re-runs when version file changes +configure_file(${PROJECT_SOURCE_DIR}/VERSION ${PROJECT_BINARY_DIR}/VERSION COPYONLY) + message( STATUS "[${PROJECT_NAME}] version ${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH} (${FULL_VERSION_STRING})" diff --git a/VERSION b/VERSION index 8f0916f768..a918a2aa18 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.5.0 +0.6.0 diff --git a/requirements.txt b/requirements.txt index 9ea6617fa7..28f19b9540 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ black clang-format>=11.0.0,<12.0.0 -clang-tidy>=15.0.0,<18.0.0 +clang-tidy>=15.0.0,<19.0.0 cmake>=3.21.0 cmake-format dataclasses diff --git a/source/include/rocprofiler-sdk/cxx/serialization.hpp b/source/include/rocprofiler-sdk/cxx/serialization.hpp index 4a633261f6..a28423802e 100644 --- a/source/include/rocprofiler-sdk/cxx/serialization.hpp +++ b/source/include/rocprofiler-sdk/cxx/serialization.hpp @@ -52,7 +52,6 @@ #include #include #include -#include #include #include #include diff --git a/source/lib/common/container/record_header_buffer.cpp b/source/lib/common/container/record_header_buffer.cpp index af9c414882..4f89afb9dd 100644 --- a/source/lib/common/container/record_header_buffer.cpp +++ b/source/lib/common/container/record_header_buffer.cpp @@ -86,8 +86,8 @@ record_header_buffer::get_record_headers(size_t _n) { auto _lk = rhb_raii_lock{*this}; - auto _sz = m_index.load(std::memory_order_acquire); - if(_n > _sz) _n = _sz; + auto _sz = m_index.load(std::memory_order_acquire); + _n = std::min(_n, _sz); auto _ret = record_ptr_vec_t{}; _ret.reserve(_n); for(size_t i = 0; i < _n; ++i) diff --git a/source/lib/common/container/small_vector.cpp b/source/lib/common/container/small_vector.cpp index b5d82010d2..37ab848abd 100644 --- a/source/lib/common/container/small_vector.cpp +++ b/source/lib/common/container/small_vector.cpp @@ -75,7 +75,7 @@ get_new_capacity(size_t min_size, size_t /*t_size*/, size_t old_capacity) // In theory 2*capacity can overflow if the capacity is 64 bit, but the // original capacity would never be large enough for this to be a problem. - size_t new_capacity = 2 * old_capacity + 1; // Always grow. + size_t new_capacity = (2 * old_capacity) + 1; // Always grow. return std::clamp(new_capacity, min_size, max_size); } } // namespace diff --git a/source/lib/rocprofiler-sdk-tool/tool.cpp b/source/lib/rocprofiler-sdk-tool/tool.cpp index e85031a16c..c118709d68 100644 --- a/source/lib/rocprofiler-sdk-tool/tool.cpp +++ b/source/lib/rocprofiler-sdk-tool/tool.cpp @@ -1311,10 +1311,8 @@ tool_init(rocprofiler_client_finalize_t fini_func, void* tool_data) auto agent_ptr_vec = get_gpu_agents(); for(auto& itr : agent_ptr_vec) { - auto method = static_cast( - tool::get_config().pc_sampling_method_value); - auto unit = static_cast( - tool::get_config().pc_sampling_unit_value); + auto method = tool::get_config().pc_sampling_method_value; + auto unit = tool::get_config().pc_sampling_unit_value; if(if_pc_sample_config_match( itr->id, method, unit, tool::get_config().pc_sampling_interval)) { diff --git a/source/lib/rocprofiler-sdk/agent.cpp b/source/lib/rocprofiler-sdk/agent.cpp index b1951e2c19..15f56d523b 100644 --- a/source/lib/rocprofiler-sdk/agent.cpp +++ b/source/lib/rocprofiler-sdk/agent.cpp @@ -631,30 +631,6 @@ get_agent_mapping() static auto*& _v = common::static_object>::construct(); return *CHECK_NOTNULL(_v); } -} // namespace - -std::vector -get_agents() -{ - auto& agents = rocprofiler::agent::get_agent_topology(); - auto pointers = std::vector{}; - pointers.reserve(agents.size()); - for(auto& agent : agents) - { - pointers.emplace_back(agent.get()); - } - return pointers; -} - -const rocprofiler_agent_t* -get_agent(rocprofiler_agent_id_t id) -{ - for(const auto& itr : get_agents()) - { - if(itr && itr->id.handle == id.handle) return itr; - } - return nullptr; -} const std::vector& get_aql_handles() @@ -682,6 +658,30 @@ get_aql_handles() return *CHECK_NOTNULL(_v); } +} // namespace + +std::vector +get_agents() +{ + auto& agents = rocprofiler::agent::get_agent_topology(); + auto pointers = std::vector{}; + pointers.reserve(agents.size()); + for(auto& agent : agents) + { + pointers.emplace_back(agent.get()); + } + return pointers; +} + +const rocprofiler_agent_t* +get_agent(rocprofiler_agent_id_t id) +{ + for(const auto& itr : get_agents()) + { + if(itr && itr->id.handle == id.handle) return itr; + } + return nullptr; +} const aqlprofile_agent_handle_t* get_aql_agent(rocprofiler_agent_id_t id) diff --git a/source/lib/rocprofiler-sdk/hsa/memory_allocation.cpp b/source/lib/rocprofiler-sdk/hsa/memory_allocation.cpp index c249bcf3ae..b3107f2e30 100644 --- a/source/lib/rocprofiler-sdk/hsa/memory_allocation.cpp +++ b/source/lib/rocprofiler-sdk/hsa/memory_allocation.cpp @@ -103,7 +103,6 @@ struct memory_allocation_info; using searchtype = SEARCHTYPE; \ auto& operator()() const { return ITERATEFUNC; } \ static constexpr auto operation_idx = ROCPROFILER_MEMORY_ALLOCATION_##ENUM; \ - static constexpr auto name = "MEMORY_ALLOCATION_" #ENUM; \ \ template \ static auto get_memory_allocation_impl(RetT (*)(Args...)) \ @@ -112,13 +111,6 @@ struct memory_allocation_info; } \ }; -SPECIALIZE_MEMORY_ALLOCATION_INFO(HSA_NONE, - NONE, - region_to_agent_map, - region_to_agent_pair, - hsa_region_t, - get_core_table()->hsa_agent_iterate_regions_fn, - memory_allocation_impl) SPECIALIZE_MEMORY_ALLOCATION_INFO(HSA_MEMORY_ALLOCATE, ALLOCATE, region_to_agent_map, @@ -425,32 +417,32 @@ get_agent(T val, IterateFunc iterate_func, CallbackFunc callback) return existing.count(val) == 0 ? null_rocp_agent_id : existing.at(val); } -void* +rocprofiler_address_t handle_starting_addr(void** starting_addr_pointer) { - return *starting_addr_pointer; + return rocprofiler_address_t{.ptr = (starting_addr_pointer) ? *starting_addr_pointer : nullptr}; } // The handle field of hsa_amd_vmem_alloc_handle_t is the starting address // cast as uint64_t, so returning the handle field after casting to void* suffices -void* +rocprofiler_address_t handle_starting_addr(hsa_amd_vmem_alloc_handle_t* vmem_alloc_handle) { - return reinterpret_cast(vmem_alloc_handle->handle); + return rocprofiler_address_t{.value = (vmem_alloc_handle) ? vmem_alloc_handle->handle : 0}; } // Handling starting address for free memory operations -void* +rocprofiler_address_t handle_starting_addr(void* starting_addr_pointer) { - return starting_addr_pointer; + return rocprofiler_address_t{.ptr = starting_addr_pointer}; } // Handles starting address for releasing handle -void* +rocprofiler_address_t handle_starting_addr(hsa_amd_vmem_alloc_handle_t vmem_alloc_handle) { - return reinterpret_cast(vmem_alloc_handle.handle); + return rocprofiler_address_t{.value = vmem_alloc_handle.handle}; } // Wrapper implementation that stores memory allocation information @@ -539,7 +531,7 @@ memory_allocation_impl(Args... args) // checks before retrieving starting address? if(starting_addr_pointer != nullptr) { - _data.address.ptr = handle_starting_addr(starting_addr_pointer); + _data.address = handle_starting_addr(starting_addr_pointer); } if(!tracing_data.empty()) @@ -584,6 +576,8 @@ memory_free_impl(Args... args) constexpr auto operation = memory_allocation_op::operation_idx; constexpr auto rocprofiler_enum = memory_allocation_info::operation_idx; + common::consume_args(arg_indices::size_idx, arg_indices::region_idx); + auto&& _tied_args = std::tie(args...); memory_allocation_data _data{}; @@ -609,7 +603,7 @@ memory_free_impl(Args... args) _data.tid = common::get_tid(); _data.func = rocprofiler_enum; _data.correlation_id = context::get_latest_correlation_id(); - _data.address.ptr = handle_starting_addr(std::get(_tied_args)); + _data.address = handle_starting_addr(std::get(_tied_args)); if(!_data.correlation_id) { diff --git a/tests/rocprofv3/CMakeLists.txt b/tests/rocprofv3/CMakeLists.txt index 7d386d4f94..c5ae31e705 100644 --- a/tests/rocprofv3/CMakeLists.txt +++ b/tests/rocprofv3/CMakeLists.txt @@ -35,4 +35,4 @@ add_subdirectory(summary) add_subdirectory(roctracer-roctx) add_subdirectory(scratch-memory) add_subdirectory(pc-sampling) -add_subdirectory(trace-period) +add_subdirectory(collection-period) diff --git a/tests/rocprofv3/trace-period/CMakeLists.txt b/tests/rocprofv3/collection-period/CMakeLists.txt similarity index 78% rename from tests/rocprofv3/trace-period/CMakeLists.txt rename to tests/rocprofv3/collection-period/CMakeLists.txt index 435335dbb8..ea26ea59f4 100644 --- a/tests/rocprofv3/trace-period/CMakeLists.txt +++ b/tests/rocprofv3/collection-period/CMakeLists.txt @@ -8,6 +8,12 @@ project( LANGUAGES CXX VERSION 0.0.0) +if(ROCPROFILER_MEMCHECK STREQUAL "ThreadSanitizer") + set(IS_THREAD_SANITIZER ON) +else() + set(IS_THREAD_SANITIZER OFF) +endif() + find_package(rocprofiler-sdk REQUIRED) string(REPLACE "LD_PRELOAD=" "ROCPROF_PRELOAD=" PRELOAD_ENV @@ -33,9 +39,16 @@ add_test( set_tests_properties( rocprofv3-test-collection-period-execute - PROPERTIES TIMEOUT 45 LABELS "integration-tests" ENVIRONMENT - "${collection-period-env}" FAIL_REGULAR_EXPRESSION - "${ROCPROFILER_DEFAULT_FAIL_REGEX}") + PROPERTIES TIMEOUT + 45 + LABELS + "integration-tests" + ENVIRONMENT + "${collection-period-env}" + FAIL_REGULAR_EXPRESSION + "${ROCPROFILER_DEFAULT_FAIL_REGEX}" + DISABLED + "${IS_THREAD_SANITIZER}") add_test( NAME rocprofv3-test-collection-period-validate @@ -47,12 +60,6 @@ add_test( --collection-period-input ${CMAKE_CURRENT_BINARY_DIR}/collection-period/out_collection_periods.log) -set(VALIDATION_FILES - ${CMAKE_CURRENT_BINARY_DIR}/collection-period/out_results.json - ${CMAKE_CURRENT_BINARY_DIR}/collection-period/out_collection_periods.log - ${CMAKE_CURRENT_BINARY_DIR}/collection-period/out_results.pftrace - ${CMAKE_CURRENT_BINARY_DIR}/collection-period/out_results.otf2) - set_tests_properties( rocprofv3-test-collection-period-validate PROPERTIES TIMEOUT @@ -63,5 +70,5 @@ set_tests_properties( "rocprofv3-test-collection-period-execute" FAIL_REGULAR_EXPRESSION "AssertionError" - ATTACHED_FILES_ON_FAIL - "${VALIDATION_FILES}") + DISABLED + "${IS_THREAD_SANITIZER}") diff --git a/tests/rocprofv3/trace-period/conftest.py b/tests/rocprofv3/collection-period/conftest.py similarity index 100% rename from tests/rocprofv3/trace-period/conftest.py rename to tests/rocprofv3/collection-period/conftest.py diff --git a/tests/rocprofv3/trace-period/pytest.ini b/tests/rocprofv3/collection-period/pytest.ini similarity index 100% rename from tests/rocprofv3/trace-period/pytest.ini rename to tests/rocprofv3/collection-period/pytest.ini diff --git a/tests/rocprofv3/trace-period/validate.py b/tests/rocprofv3/collection-period/validate.py similarity index 100% rename from tests/rocprofv3/trace-period/validate.py rename to tests/rocprofv3/collection-period/validate.py