Linting workflow and clang-tidy fixes (#72)

* Update source/{bin,lib/{common,rocprofiler}}/CMakeLists.txt

- activate clang-tidy

* Update PTL submodule

- clang-tidy fixes

* Update .clang-tidy

- ignore performance-enum-size

* Update CI workflow

- update paths-ignore

* Add linting workflow

- runs clang-tidy

* Update cmake/rocprofiler_build_settings.cmake

- minor modification of flags not recognized by clang-tidy

* Update samples (all of them)

- rocprofiler-samples-build-flags target with -W -Wall -Wextra -Wshadow [-Werror]
- Link samples targets to rocprofiler-samples-build-flags if target exists
- Remove unused variable in main.cpp of api_{buffered,callback}_tracing
- Update samples/pc_sampling
  - single-user-multiple-agents.cpp ends up with unused function find_first_gpu_agent() error
  - change find_first_gpu_agent to return std::optional<rocprofiler_agent_t>
  - change usage after call to find_first_gpu_agent()
  - use find_first_gpu_agent() in single-user-multiple-agents.cpp to determine if there are any GPUs

* Update linting workflow

- fix path to run-ci.py script

* Update linting workflow

- install cmake

* Update common/container/stable_vector.hpp

- fix clang-tidy warning for readability-container-size-empty

.clang-tidy

@@ -24,6 +24,7 @@ modernize-*,\
 -modernize-macro-to-enum,\
 -modernize-type-traits,\
 performance-*,\
+-performance-enum-size,\
 readability-*,\
 -readability-function-size,\
 -readability-identifier-naming,\

.github/workflows/continuous_integration.yml

@@ -5,17 +5,13 @@ on:
   push:
     branches: [ "main" ]
     paths-ignore:
-      - '.github/workflows/pull_*.yml'
-      - '.github/workflows/linting.yml'
-      - '.github/workflows/markdown_lint.yml'
       - '*.md'
+      - 'source/docs/**'
   pull_request:
     branches: [ "main" ]
     paths-ignore:
-      - '.github/workflows/pull_*.yml'
-      - '.github/workflows/linting.yml'
-      - '.github/workflows/markdown_lint.yml'
       - '*.md'
+      - 'source/docs/**'
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}

.github/workflows/linting.yml

@@ -0,0 +1,86 @@
+name: Linting
+
+on:
+  workflow_dispatch:
+  push:
+    branches: [ "main" ]
+    paths-ignore:
+      - '*.md'
+      - 'source/docs/**'
+  pull_request:
+    branches: [ "main" ]
+    paths-ignore:
+      - '*.md'
+      - 'source/docs/**'
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+env:
+  # TODO(jrmadsen): replace LD_RUNPATH_FLAG, GPU_LIST, etc. with internal handling in cmake
+  ROCM_PATH: "/opt/rocm"
+  GPU_LIST: "gfx900 gfx906 gfx908 gfx90a gfx940 gfx941 gfx942 gfx1030 gfx1100 gfx1101 gfx1102"
+  PATH: "/usr/bin:$PATH"
+
+jobs:
+  get-latest-mainline-build-number:
+    runs-on: mi200
+
+    outputs:
+      LATEST_BUILD_NUMBER: ${{ steps.get_build_number.outputs.LATEST_BUILD_NUMBER }}
+
+    steps:
+      - id: get_build_number
+        run: echo "LATEST_BUILD_NUMBER=$(wget -qO- 'http://rocm-ci.amd.com/job/compute-rocm-dkms-no-npi-hipclang/lastSuccessfulBuild/buildNumber')" >> $GITHUB_OUTPUT
+
+  clang-tidy:
+    strategy:
+      fail-fast: false
+      matrix:
+        os: ['ubuntu-22.04']
+        runner: ['renderD131']
+        device: ['/renderD131']
+        linter: ['clang-tidy']
+
+    runs-on: ${{ matrix.runner }}
+
+    # define this for containers
+    env:
+      GIT_DISCOVERY_ACROSS_FILESYSTEM: 1
+
+    container:
+      image: compute-artifactory.amd.com:5000/rocm-plus-docker/compute-rocm-dkms-no-npi-hipclang:${{ needs.get-latest-mainline-build-number.outputs.LATEST_BUILD_NUMBER }}-${{ matrix.os }}-stg1
+      options: --privileged --ipc=host --device=/dev/kfd --device=/dev/dri${{ matrix.device }} --group-add video --cap-add=SYS_PTRACE --cap-add CAP_SYS_PTRACE --cap-add CAP_SYS_ADMIN --security-opt seccomp=unconfined
+
+    if: ${{ always() }}
+    needs: get-latest-mainline-build-number
+
+    steps:
+    - uses: actions/checkout@v4
+      with:
+        submodules: true
+
+    - name: Update container
+      run: |
+        apt-get update
+        apt-get install -y cmake clang-tidy-11 g++-11 g++-12
+        update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-11 10
+        update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-11 10 --slave /usr/bin/g++ g++ /usr/bin/g++-11
+        update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-12 20 --slave /usr/bin/g++ g++ /usr/bin/g++-12
+    - name: Configure, Build, and Test
+      timeout-minutes: 30
+      shell: bash
+      run:
+        python3 ./source/scripts/run-ci.py -B build
+          --name ${{ github.repository }}-${{ github.ref_name }}-mi200-${{ matrix.linter }}
+          --build-jobs 8
+          --site mi200
+          --gpu-targets ${{ env.GPU_LIST }}
+          --linter ${{ matrix.linter }}
+          --
+          -DCMAKE_BUILD_TYPE=Release
+          -DCMAKE_INSTALL_PREFIX="${{ env.ROCM_PATH }}"
+          -DPython3_EXECUTABLE=$(which python3)
+          --
+          -VV

cmake/rocprofiler_build_settings.cmake

@@ -37,7 +37,7 @@ foreach(_TYPE dl rt)
             rocprofiler_target_compile_definitions(rocprofiler-${_TYPE}
                                                    INTERFACE ROCPROFILER_${_TYPE_UC}=1)
             target_link_libraries(rocprofiler-${_TYPE} INTERFACE ${${_TYPE}_LIBRARY})
-            if("${_TYPE}" STREQUAL "dl")
+            if("${_TYPE}" STREQUAL "dl" AND NOT ROCPROFILER_ENABLE_CLANG_TIDY)
                 # This instructs the linker to add all symbols, not only used ones, to the
                 # dynamic symbol table. This option is needed for some uses of dlopen or
                 # to allow obtaining backtraces from within a program.
@@ -120,9 +120,7 @@ endif()
 rocprofiler_target_compile_options(
     rocprofiler-developer-flags
     LANGUAGES C CXX
-    INTERFACE "-Werror" "-Wdouble-promotion" "-Wshadow" "-Wextra"
-              "-Wstack-usage=524288" # 512 KB
-    )
+    INTERFACE "-Werror" "-Wdouble-promotion" "-Wshadow" "-Wextra")
 
 if(ROCPROFILER_BUILD_DEVELOPER)
     target_link_libraries(rocprofiler-build-flags

samples/CMakeLists.txt

@@ -5,6 +5,15 @@ cmake_minimum_required(VERSION 3.21.0 FATAL_ERROR)
 
 project(rocprofiler-samples LANGUAGES C CXX)
 
+add_library(rocprofiler-samples-build-flags INTERFACE)
+add_library(rocprofiler::samples-build-flags ALIAS rocprofiler-samples-build-flags)
+target_compile_options(rocprofiler-samples-build-flags INTERFACE -W -Wall -Wextra
+                                                                 -Wshadow)
+
+if(ROCPROFILER_BUILD_CI OR ROCPROFILER_BUILD_WERROR)
+    target_compile_options(rocprofiler-samples-build-flags INTERFACE -Werror)
+endif()
+
 # add_subdirectory(api_tracing)
 add_subdirectory(pc_sampling)
 add_subdirectory(api_callback_tracing)

samples/api_buffered_tracing/CMakeLists.txt

@@ -31,16 +31,20 @@ endif()
 
 add_library(buffered-api-tracing-client SHARED)
 target_sources(buffered-api-tracing-client PRIVATE client.cpp client.hpp)
-target_link_libraries(buffered-api-tracing-client
-                      PRIVATE rocprofiler::rocprofiler-library)
+target_link_libraries(
+    buffered-api-tracing-client
+    PRIVATE rocprofiler::rocprofiler-library
+            $<TARGET_NAME_IF_EXISTS:rocprofiler::samples-build-flags>)
 
 set_source_files_properties(main.cpp PROPERTIES LANGUAGE HIP)
 find_package(Threads REQUIRED)
 
 add_executable(buffered-api-tracing)
 target_sources(buffered-api-tracing PRIVATE main.cpp)
-target_link_libraries(buffered-api-tracing PRIVATE buffered-api-tracing-client
-                                                   Threads::Threads)
+target_link_libraries(
+    buffered-api-tracing
+    PRIVATE buffered-api-tracing-client Threads::Threads
+            $<TARGET_NAME_IF_EXISTS:rocprofiler::samples-build-flags>)
 
 add_test(NAME buffered-api-tracing COMMAND $<TARGET_FILE:buffered-api-tracing>)
 

samples/api_buffered_tracing/main.cpp

@@ -76,7 +76,6 @@ main(int argc, char** argv)
     client::start();  // starts context before any API tables are available
 
     int rank = 0;
-    int size = 1;
     for(int i = 1; i < argc; ++i)
     {
         auto _arg = std::string{argv[i]};

samples/api_callback_tracing/CMakeLists.txt

@@ -31,16 +31,20 @@ endif()
 
 add_library(callback-api-tracing-client SHARED)
 target_sources(callback-api-tracing-client PRIVATE client.cpp client.hpp)
-target_link_libraries(callback-api-tracing-client
-                      PRIVATE rocprofiler::rocprofiler-library)
+target_link_libraries(
+    callback-api-tracing-client
+    PRIVATE rocprofiler::rocprofiler-library
+            $<TARGET_NAME_IF_EXISTS:rocprofiler::samples-build-flags>)
 
 set_source_files_properties(main.cpp PROPERTIES LANGUAGE HIP)
 find_package(Threads REQUIRED)
 
 add_executable(callback-api-tracing)
 target_sources(callback-api-tracing PRIVATE main.cpp)
-target_link_libraries(callback-api-tracing PRIVATE callback-api-tracing-client
-                                                   Threads::Threads)
+target_link_libraries(
+    callback-api-tracing
+    PRIVATE callback-api-tracing-client Threads::Threads
+            $<TARGET_NAME_IF_EXISTS:rocprofiler::samples-build-flags>)
 
 add_test(NAME callback-api-tracing COMMAND $<TARGET_FILE:callback-api-tracing>)
 

samples/api_callback_tracing/main.cpp

@@ -76,7 +76,6 @@ main(int argc, char** argv)
     // client::start(); // currently will fail
 
     int rank = 0;
-    int size = 1;
     for(int i = 1; i < argc; ++i)
     {
         auto _arg = std::string{argv[i]};

samples/pc_sampling/CMakeLists.txt

@@ -12,17 +12,23 @@ endif()
 add_executable(pc_sampling_single-user-host-trap)
 target_sources(pc_sampling_single-user-host-trap PRIVATE common.h
                                                          single-user-host-trap.cpp)
-target_link_libraries(pc_sampling_single-user-host-trap
-                      PRIVATE rocprofiler::rocprofiler-library)
+target_link_libraries(
+    pc_sampling_single-user-host-trap
+    PRIVATE rocprofiler::rocprofiler-library
+            $<TARGET_NAME_IF_EXISTS:rocprofiler::samples-build-flags>)
 
 add_executable(pc_sampling_single-user-host-trap-retry)
 target_sources(pc_sampling_single-user-host-trap-retry
                PRIVATE common.h single-user-host-trap-retries-service-instantiation.cpp)
-target_link_libraries(pc_sampling_single-user-host-trap-retry
-                      PRIVATE rocprofiler::rocprofiler-library)
+target_link_libraries(
+    pc_sampling_single-user-host-trap-retry
+    PRIVATE rocprofiler::rocprofiler-library
+            $<TARGET_NAME_IF_EXISTS:rocprofiler::samples-build-flags>)
 
 add_executable(pc_sampling_single-user-multiple-agents)
 target_sources(pc_sampling_single-user-multiple-agents
                PRIVATE common.h single-user-multiple-agents.cpp)
-target_link_libraries(pc_sampling_single-user-multiple-agents
-                      PRIVATE rocprofiler::rocprofiler-library)
+target_link_libraries(
+    pc_sampling_single-user-multiple-agents
+    PRIVATE rocprofiler::rocprofiler-library
+            $<TARGET_NAME_IF_EXISTS:rocprofiler::samples-build-flags>)

samples/pc_sampling/common.h

@@ -7,6 +7,7 @@
 #include <cstdint>
 #include <cstdio>
 #include <cstdlib>
+#include <optional>
 #include <string>
 #include <string_view>
 
@@ -69,7 +70,7 @@ find_first_gpu_agent_impl(rocprofiler_agent_t** agents, size_t num_agents, void*
     return ROCPROFILER_STATUS_ERROR;
 }
 
-static rocprofiler_agent_t
+static std::optional<rocprofiler_agent_t>
 find_first_gpu_agent()
 {
     // This function returns the first gpu agent it encounters.
@@ -77,10 +78,10 @@ find_first_gpu_agent()
     //   and return if the agent is MI200.
     rocprofiler_agent_t gpu_agent;
 
-    ROCPROFILER_CALL(rocprofiler_query_available_agents(&find_first_gpu_agent_impl,
-                                                        sizeof(rocprofiler_agent_t),
-                                                        static_cast<void*>(&gpu_agent)),
-                     "Could not query GPU agents");
+    auto status = rocprofiler_query_available_agents(
+        &find_first_gpu_agent_impl, sizeof(rocprofiler_agent_t), static_cast<void*>(&gpu_agent));
+
+    if(status != ROCPROFILER_STATUS_SUCCESS) return std::nullopt;
 
     return gpu_agent;
 }

samples/pc_sampling/single-user-host-trap-retries-service-instantiation.cpp

@@ -10,6 +10,8 @@
 #include "common.h"
 
 #include <cassert>
+#include <cstdlib>
+#include <stdexcept>
 #include <vector>
 
 #define HOST_TRAP_INTERVAL 1000
@@ -26,7 +28,8 @@ second_user()
     ROCPROFILER_CALL(rocprofiler_create_context(&context_id2),
                      "Cannot create context for the second user\n");
 
-    rocprofiler_agent_t gpu_agent = find_first_gpu_agent();
+    auto gpu_agent = find_first_gpu_agent();
+    if(!gpu_agent) throw std::runtime_error{"no gpu agents were found"};
 
     // creating a buffer that will hold pc sampling information
     rocprofiler_buffer_policy_t lossless_buffer_action = ROCPROFILER_BUFFER_POLICY_LOSSLESS;
@@ -49,7 +52,7 @@ second_user()
     // configured.
     ROCPROFILER_CALL_FAILS(
         rocprofiler_configure_pc_sampling_service(
-            context_id2, gpu_agent, sampling_method2, sampling_unit2, interval2, buffer_id2),
+            context_id2, *gpu_agent, sampling_method2, sampling_unit2, interval2, buffer_id2),
         "Instantiation of the PC sampling service should fail");
 
     // After failure, the second user queries available configuration and observes the one chosen by
@@ -57,7 +60,7 @@ second_user()
     size_t                                               config_count = 10;
     std::vector<rocprofiler_pc_sampling_configuration_t> configs(config_count);
     ROCPROFILER_CALL(rocprofiler_query_pc_sampling_agent_configurations(
-                         gpu_agent, configs.data(), &config_count),
+                         *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
@@ -76,7 +79,7 @@ second_user()
     // The second user is satisfied with the configuration chosen by the first user, so it
     // starts PC sampling.
     ROCPROFILER_CALL(rocprofiler_configure_pc_sampling_service(context_id2,
-                                                               gpu_agent,
+                                                               *gpu_agent,
                                                                first_user_config.method,
                                                                first_user_config.unit,
                                                                first_user_config.min_interval,
@@ -108,7 +111,12 @@ main(int /*argc*/, char** /*argv*/)
     rocprofiler_context_id_t context_id;
     ROCPROFILER_CALL(rocprofiler_create_context(&context_id), "Cannot create context\n");
 
-    rocprofiler_agent_t gpu_agent = find_first_gpu_agent();
+    auto gpu_agent = find_first_gpu_agent();
+    if(!gpu_agent)
+    {
+        fprintf(stderr, "no gpu agents were found\n");
+        return EXIT_FAILURE;
+    }
 
     // creating a buffer that will hold pc sampling information
     rocprofiler_buffer_policy_t drop_buffer_action = ROCPROFILER_BUFFER_POLICY_DISCARD;
@@ -129,7 +137,7 @@ main(int /*argc*/, char** /*argv*/)
     host_trap_interval = HOST_TRAP_INTERVAL;
     // Instantiating the first PC sampling service succeeds.
     ROCPROFILER_CALL(rocprofiler_configure_pc_sampling_service(context_id,
-                                                               gpu_agent,
+                                                               *gpu_agent,
                                                                host_trap_sampling_method,
                                                                host_trap_sampling_unit_time,
                                                                host_trap_interval,

samples/pc_sampling/single-user-host-trap.cpp

@@ -2,6 +2,7 @@
 // If any of the rocprofiler calls returns status fail, we simply stop the application.
 
 #include <rocprofiler/rocprofiler.h>
+#include <cstdlib>
 #include "common.h"
 
 int
@@ -11,7 +12,12 @@ main(int /*argc*/, char** /*argv*/)
     rocprofiler_context_id_t context_id;
     ROCPROFILER_CALL(rocprofiler_create_context(&context_id), "Cannot create context\n");
 
-    rocprofiler_agent_t gpu_agent = find_first_gpu_agent();
+    auto gpu_agent = find_first_gpu_agent();
+    if(!gpu_agent)
+    {
+        fprintf(stderr, "no gpu agents were found\n");
+        return EXIT_FAILURE;
+    }
 
     // creating a buffer that will hold pc sampling information
     rocprofiler_buffer_policy_t drop_buffer_action = ROCPROFILER_BUFFER_POLICY_DISCARD;
@@ -33,7 +39,7 @@ main(int /*argc*/, char** /*argv*/)
     // Instantiating the PC sampling service
     ROCPROFILER_CALL(
         rocprofiler_configure_pc_sampling_service(
-            context_id, gpu_agent, sampling_method, sampling_unit, interval, buffer_id),
+            context_id, *gpu_agent, sampling_method, sampling_unit, interval, buffer_id),
         "Cannot create PC sampling service");
 
     // Vladimir: Is this the place of retrying if someone already created the

samples/pc_sampling/single-user-multiple-agents.cpp

@@ -64,7 +64,6 @@ void
 find_all_gpu_agents_supporting_pc_sampling()
 {
     // This function returns the all gpu agents supporting some kind of PC sampling
-    std::vector<rocprofiler_agent_t> gpu_agents;
     ROCPROFILER_CALL(
         rocprofiler_query_available_agents(&find_all_gpu_agents_supporting_pc_sampling_impl,
                                            sizeof(rocprofiler_agent_t),
@@ -101,7 +100,7 @@ extract_stochastic_config(rocprofiler_pc_sampling_config_array_t* configs)
 {
     // Iterate over an array of configurations and return the first one
     // with stochasting method.
-    for(int i = 0; i < configs->size; i++)
+    for(size_t i = 0; i < configs->size; i++)
     {
         if(configs->data[i].method == ROCPROFILER_PC_SAMPLING_METHOD_STOCHASTIC)
         {
@@ -136,7 +135,11 @@ configure_stochastic_sampling(rocprofiler_context_id_t context_id,
 int
 main(int /*argc*/, char** /*argv*/)
 {
-    rocprofiler_status_t status;
+    if(!find_first_gpu_agent())
+    {
+        fprintf(stderr, "no gpu agents were found\n");
+        return EXIT_FAILURE;
+    }
 
     find_all_gpu_agents_supporting_pc_sampling();
 
@@ -185,7 +188,7 @@ main(int /*argc*/, char** /*argv*/)
     // Running the applicaiton
     run_HIP_app();
 
-    for(int i = 0; i < gpu_agents.size(); i++)
+    for(size_t i = 0; i < gpu_agents.size(); i++)
     {
         // Stop the context that should stop PC sampling?
         ROCPROFILER_CALL(rocprofiler_stop_context(contexts[i]), "Cannot start PC sampling context");

source/bin/CMakeLists.txt

@@ -1 +1,5 @@
+#
+#
+#
 
+rocprofiler_activate_clang_tidy()

source/lib/common/CMakeLists.txt

@@ -1,6 +1,8 @@
 #
 #   Builds common utilities into a static library
 #
+rocprofiler_activate_clang_tidy()
+
 set(common_sources config.cpp environment.cpp demangle.cpp)
 set(common_headers config.hpp defines.hpp environment.hpp demangle.hpp mpl.hpp
                    utility.hpp)

source/lib/common/container/stable_vector.hpp

@@ -195,7 +195,7 @@ public:
     size_type max_size() const noexcept { return std::numeric_limits<size_type>::max(); }
     size_type capacity() const noexcept { return m_chunks.size() * ChunkSizeV; }
 
-    bool empty() const noexcept { return m_chunks.size() == 0; }
+    bool empty() const noexcept { return m_chunks.empty(); }
 
     void reserve(size_type new_capacity);
     void shrink_to_fit() noexcept {}

source/lib/rocprofiler/CMakeLists.txt

@@ -1,6 +1,8 @@
 #
 #
 #
+rocprofiler_activate_clang_tidy()
+
 set(ROCPROFILER_LIB_HEADERS buffer.hpp internal_threading.hpp registration.hpp)
 set(ROCPROFILER_LIB_SOURCES
     buffer.cpp buffer_tracing.cpp callback_tracing.cpp context.cpp internal_threading.cpp