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 <[email protected]>
ROCm · Dec 6, 2024 · bd447ab · bd447ab
1 parent f18158f
commit bd447ab
Show file tree

Hide file tree

Showing 14 changed files with 65 additions and 64 deletions.
diff --git 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
@@ -1 +1 @@
-0.5.0
+0.6.0
diff --git 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
@@ -52,7 +52,6 @@
 #include <cereal/types/set.hpp>
 #include <cereal/types/stack.hpp>
 #include <cereal/types/string.hpp>
-#include <cereal/types/tuple.hpp>
 #include <cereal/types/unordered_map.hpp>
 #include <cereal/types/unordered_set.hpp>
 #include <cereal/types/utility.hpp>

diff --git 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
@@ -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
@@ -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<rocprofiler_pc_sampling_method_t>(
-                tool::get_config().pc_sampling_method_value);
-            auto unit = static_cast<rocprofiler_pc_sampling_unit_t>(
-                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
@@ -631,30 +631,6 @@ get_agent_mapping()
     static auto*& _v = common::static_object<std::vector<agent_pair>>::construct();
     return *CHECK_NOTNULL(_v);
 }
-}  // namespace
-
-std::vector<const rocprofiler_agent_t*>
-get_agents()
-{
-    auto& agents   = rocprofiler::agent::get_agent_topology();
-    auto  pointers = std::vector<const rocprofiler_agent_t*>{};
-    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<aqlprofile_agent_handle_t>&
 get_aql_handles()
@@ -682,6 +658,30 @@ get_aql_handles()
 
     return *CHECK_NOTNULL(_v);
 }
+}  // namespace
+
+std::vector<const rocprofiler_agent_t*>
+get_agents()
+{
+    auto& agents   = rocprofiler::agent::get_agent_topology();
+    auto  pointers = std::vector<const rocprofiler_agent_t*>{};
+    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
@@ -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 <size_t TableIdx, size_t OpIdx, typename RetT, typename... Args>                  \
         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<void*>(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<void*>(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<OpIdx>::operation_idx;
     constexpr auto rocprofiler_enum = memory_allocation_info<operation>::operation_idx;
 
+    common::consume_args(arg_indices<OpIdx>::size_idx, arg_indices<OpIdx>::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<address_idx>(_tied_args));
+    _data.address        = handle_starting_addr(std::get<address_idx>(_tied_args));
 
     if(!_data.correlation_id)
     {

diff --git 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 → ...ocprofv3/collection-period/CMakeLists.txt b/tests/rocprofv3/trace-period/CMakeLists.txt → ...ocprofv3/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 → ...s/rocprofv3/collection-period/conftest.py b/tests/rocprofv3/trace-period/conftest.py → ...s/rocprofv3/collection-period/conftest.py
diff --git a/tests/rocprofv3/trace-period/pytest.ini → tests/rocprofv3/collection-period/pytest.ini b/tests/rocprofv3/trace-period/pytest.ini → tests/rocprofv3/collection-period/pytest.ini
diff --git a/tests/rocprofv3/trace-period/validate.py → ...s/rocprofv3/collection-period/validate.py b/tests/rocprofv3/trace-period/validate.py → ...s/rocprofv3/collection-period/validate.py