From 070b659a9ae999747bd70959cce2f0f652463d7a Mon Sep 17 00:00:00 2001 From: "Madsen, Jonathan" Date: Tue, 11 Feb 2025 10:58:47 -0600 Subject: [PATCH] Re-enable clang-tidy for core workflows + clang-tidy fixes (#197) * Ensure the clang-tidy is updated + clang-tidy fixes * update-ci workflow * Enable clang-tidy checks * Add extra logging to device counter collection samples * Misc clang-tidy fixes * Disable device counter collection samples for ThreadSanitizer * Formatting --------- Co-authored-by: Jonathan R. Madsen --- .github/workflows/continuous_integration.yml | 14 ++++---- samples/counter_collection/CMakeLists.txt | 32 +++++++++++++++---- .../counter_collection/device_counting.cpp | 5 ++- .../device_counting_synchronous.cpp | 4 +++ source/lib/output/generateCSV.cpp | 2 +- source/lib/output/generator.hpp | 2 +- .../rocprofiler-sdk-att/att_lib_wrapper.cpp | 2 +- .../rocprofiler-sdk-att/att_lib_wrapper.hpp | 6 ++-- source/lib/rocprofiler-sdk-att/occupancy.cpp | 10 +++--- source/lib/rocprofiler-sdk-att/outputfile.hpp | 2 +- .../rocprofiler-sdk-att/profile_interface.cpp | 6 ++-- .../rocprofiler-sdk-att/profile_interface.hpp | 4 +-- .../lib/rocprofiler-sdk-att/waitcnt/gfx10.cpp | 5 ++- .../lib/rocprofiler-sdk-att/waitcnt/gfx12.cpp | 5 ++- source/lib/rocprofiler-sdk-att/wave.cpp | 8 ++--- source/lib/rocprofiler-sdk-roctx/roctx.cpp | 2 +- .../kernel_dispatch/profiling_time.cpp | 2 +- .../page_migration/page_migration.cpp | 4 +-- 18 files changed, 76 insertions(+), 39 deletions(-) diff --git a/.github/workflows/continuous_integration.yml b/.github/workflows/continuous_integration.yml index a350c53376..fe1cf24821 100644 --- a/.github/workflows/continuous_integration.yml +++ b/.github/workflows/continuous_integration.yml @@ -68,7 +68,7 @@ jobs: apt-get install -y build-essential cmake g++-11 g++-12 python3-pip libdw-dev rccl-dev rccl-unittests update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-11 10 --slave /usr/bin/g++ g++ /usr/bin/g++-11 --slave /usr/bin/gcov gcov /usr/bin/gcov-11 update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-12 20 --slave /usr/bin/g++ g++ /usr/bin/g++-12 --slave /usr/bin/gcov gcov /usr/bin/gcov-12 - python3 -m pip install -r requirements.txt + python3 -m pip install -U --user -r requirements.txt - name: List Files shell: bash @@ -96,6 +96,7 @@ jobs: --site ${RUNNER_HOSTNAME} --gpu-targets ${{ env.GPU_TARGETS }} --run-attempt ${{ github.run_attempt }} + ${{ matrix.ci-flags }} -- -DROCPROFILER_DEP_ROCMCORE=ON -DROCPROFILER_BUILD_DOCS=OFF @@ -190,8 +191,7 @@ jobs: timeout-minutes: 10 shell: bash run: | - sudo python3 -m pip install 'perfetto<0.9.0' - sudo python3 -m pip install -r requirements.txt + sudo python3 -m pip install -U --user -r requirements.txt - name: List Files shell: bash @@ -220,6 +220,7 @@ jobs: --site ${RUNNER_HOSTNAME} --gpu-targets ${{ env.GPU_TARGETS }} --run-attempt ${{ github.run_attempt }} + ${{ matrix.ci-flags }} -- -DROCPROFILER_DEP_ROCMCORE=ON -DROCPROFILER_BUILD_DOCS=OFF @@ -346,7 +347,7 @@ jobs: git config --global --add safe.directory '*' apt-get update apt-get install -y build-essential cmake python3-pip gcovr wkhtmltopdf xvfb xfonts-base xfonts-75dpi xfonts-100dpi xfonts-utils xfonts-encodings libfontconfig libdw-dev - python3 -m pip install -r requirements.txt + python3 -m pip install -U --user -r requirements.txt - name: Sync gcov with compilers timeout-minutes: 10 @@ -608,12 +609,13 @@ jobs: run: | git config --global --add safe.directory '*' apt-get update - apt-get install -y build-essential cmake python3-pip libasan8 libtsan2 software-properties-common + apt-get install -y build-essential cmake python3-pip libasan8 libtsan2 software-properties-common clang-15 add-apt-repository ppa:ubuntu-toolchain-r/test apt-get update + apt-get upgrade -y apt-get install -y gcc-${{ env.GCC_COMPILER_VERSION }} g++-${{ env.GCC_COMPILER_VERSION }} libdw-dev update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-${{ env.GCC_COMPILER_VERSION }} 100 --slave /usr/bin/g++ g++ /usr/bin/g++-${{ env.GCC_COMPILER_VERSION }} --slave /usr/bin/gcov gcov /usr/bin/gcov-${{ env.GCC_COMPILER_VERSION }} - python3 -m pip install -r requirements.txt + python3 -m pip install -U --user -r requirements.txt - name: List Files shell: bash diff --git a/samples/counter_collection/CMakeLists.txt b/samples/counter_collection/CMakeLists.txt index cd90b92118..ecf35bbbf3 100644 --- a/samples/counter_collection/CMakeLists.txt +++ b/samples/counter_collection/CMakeLists.txt @@ -25,6 +25,12 @@ foreach(_TYPE DEBUG MINSIZEREL RELEASE RELWITHDEBINFO) endif() endforeach() +if(ROCPROFILER_MEMCHECK STREQUAL "ThreadSanitizer") + set(IS_THREAD_SANITIZER ON) +else() + set(IS_THREAD_SANITIZER OFF) +endif() + find_package(rocprofiler-sdk REQUIRED) add_library(counter-collection-buffer-client SHARED) @@ -152,9 +158,16 @@ add_test(NAME counter-collection-device-profiling set_tests_properties( counter-collection-device-profiling - PROPERTIES TIMEOUT 120 LABELS "samples" ENVIRONMENT - "${counter-collection-functional-counter-env}" FAIL_REGULAR_EXPRESSION - "${ROCPROFILER_DEFAULT_FAIL_REGEX}") + PROPERTIES TIMEOUT + 120 + LABELS + "samples" + ENVIRONMENT + "${counter-collection-functional-counter-env}" + FAIL_REGULAR_EXPRESSION + "${ROCPROFILER_DEFAULT_FAIL_REGEX}" + DISABLED + "${IS_THREAD_SANITIZER}") add_library(counter-collection-device-profiling-sync-client SHARED) target_sources(counter-collection-device-profiling-sync-client @@ -180,6 +193,13 @@ add_test(NAME counter-collection-device-profiling-sync set_tests_properties( counter-collection-device-profiling-sync - PROPERTIES TIMEOUT 120 LABELS "samples" ENVIRONMENT - "${counter-collection-functional-counter-env}" FAIL_REGULAR_EXPRESSION - "${ROCPROFILER_DEFAULT_FAIL_REGEX}") + PROPERTIES TIMEOUT + 120 + LABELS + "samples" + ENVIRONMENT + "${counter-collection-functional-counter-env}" + FAIL_REGULAR_EXPRESSION + "${ROCPROFILER_DEFAULT_FAIL_REGEX}" + DISABLED + "${IS_THREAD_SANITIZER}") diff --git a/samples/counter_collection/device_counting.cpp b/samples/counter_collection/device_counting.cpp index 0e0d39496d..5d2cd6e06c 100644 --- a/samples/counter_collection/device_counting.cpp +++ b/samples/counter_collection/device_counting.cpp @@ -307,17 +307,20 @@ tool_init(rocprofiler_client_finalize_t, void* user_data) void tool_fini(void* user_data) { + std::clog << "In tool fini\n" << std::flush; + exit_toggle().store(true); while(exit_toggle().load() == true) {}; - std::clog << "In tool fini\n"; rocprofiler_stop_context(get_client_ctx()); ROCPROFILER_CALL(rocprofiler_flush_buffer(get_buffer()), "buffer flush"); auto* output_stream = static_cast(user_data); *output_stream << std::flush; if(output_stream != &std::cout && output_stream != &std::cerr) delete output_stream; + + std::clog << "Completed tool fini\n" << std::flush; } } // namespace diff --git a/samples/counter_collection/device_counting_synchronous.cpp b/samples/counter_collection/device_counting_synchronous.cpp index 80e24dc5af..4e90b68ce1 100644 --- a/samples/counter_collection/device_counting_synchronous.cpp +++ b/samples/counter_collection/device_counting_synchronous.cpp @@ -424,6 +424,8 @@ tool_init(rocprofiler_client_finalize_t fini_func, void*) void tool_fini(void* user_data) { + std::clog << "In tool fini\n" << std::flush; + client_id = nullptr; exit_toggle().store(true); @@ -441,6 +443,8 @@ tool_fini(void* user_data) sampler.reset(); delete sampler_thread; + + std::clog << "Completed tool fini\n" << std::flush; } extern "C" rocprofiler_tool_configure_result_t* diff --git a/source/lib/output/generateCSV.cpp b/source/lib/output/generateCSV.cpp index 48c6c98b82..386a0e28cc 100644 --- a/source/lib/output/generateCSV.cpp +++ b/source/lib/output/generateCSV.cpp @@ -585,7 +585,7 @@ generate_csv(const output_config& cfg, for(auto ditr : data) { - for(auto record : data.get(ditr)) + for(const auto& record : data.get(ditr)) { auto kernel_id = record.dispatch_data.dispatch_info.kernel_id; auto counter_id_value = std::map{}; diff --git a/source/lib/output/generator.hpp b/source/lib/output/generator.hpp index 7ee45a8eb5..61213e4236 100644 --- a/source/lib/output/generator.hpp +++ b/source/lib/output/generator.hpp @@ -146,7 +146,7 @@ save(ArchiveT& ar, const rocprofiler::tool::generator& data) for(auto itr : data) { auto dat = data.get(itr); - for(auto ditr : dat) + for(const auto& ditr : dat) ar(ditr); } } diff --git a/source/lib/rocprofiler-sdk-att/att_lib_wrapper.cpp b/source/lib/rocprofiler-sdk-att/att_lib_wrapper.cpp index dcc4e9cd07..bfd3c0ba15 100644 --- a/source/lib/rocprofiler-sdk-att/att_lib_wrapper.cpp +++ b/source/lib/rocprofiler-sdk-att/att_lib_wrapper.cpp @@ -147,7 +147,7 @@ ATTDecoder::parse(const Fspath& input_dir, mgr.table->addDecoder((input_dir / file.name).c_str(), file.id, file.addr, file.size); } catch(std::exception& e) { - ROCP_ERROR << file.id << ':' << file.name << " - " << e.what() << std::endl; + ROCP_ERROR << file.id << ':' << file.name << " - " << e.what(); } } diff --git a/source/lib/rocprofiler-sdk-att/att_lib_wrapper.hpp b/source/lib/rocprofiler-sdk-att/att_lib_wrapper.hpp index 9444526865..033db690b2 100644 --- a/source/lib/rocprofiler-sdk-att/att_lib_wrapper.hpp +++ b/source/lib/rocprofiler-sdk-att/att_lib_wrapper.hpp @@ -47,9 +47,9 @@ struct CodeobjLoadInfo enum tool_att_capability_t { - ATT_CAPABILITIES_TESTING, // used for code coverage testing - ATT_CAPABILITIES_SUMMARY, // used for CSV output only - ATT_CAPABILITIES_TRACE, // used for all outputs + ATT_CAPABILITIES_TESTING = 0, // used for code coverage testing + ATT_CAPABILITIES_SUMMARY, // used for CSV output only + ATT_CAPABILITIES_TRACE, // used for all outputs ATT_CAPABILITIES_DEBUG, ATT_CAPABILITIES_LAST = ATT_CAPABILITIES_DEBUG, }; diff --git a/source/lib/rocprofiler-sdk-att/occupancy.cpp b/source/lib/rocprofiler-sdk-att/occupancy.cpp index 90c567e8dd..90b9d16b8d 100644 --- a/source/lib/rocprofiler-sdk-att/occupancy.cpp +++ b/source/lib/rocprofiler-sdk-att/occupancy.cpp @@ -35,6 +35,8 @@ namespace rocprofiler { namespace att_wrapper { +namespace +{ union occupancy_data_v1 { struct @@ -52,7 +54,7 @@ union occupancy_data_v1 std::map kernel_ids{{pcinfo_t{0, 0}, 0}}; std::atomic current_id{1}; -static int +int get_kernel_id(pcinfo_t pc) { if(kernel_ids.find(pc) != kernel_ids.end()) return kernel_ids.at(pc); @@ -60,7 +62,7 @@ get_kernel_id(pcinfo_t pc) return kernel_ids.emplace(pc, current_id.fetch_add(1)).first->second; } -static uint64_t +uint64_t convert(const att_occupancy_info_v2_t& v2) { occupancy_data_v1 v1{}; @@ -71,7 +73,8 @@ convert(const att_occupancy_info_v2_t& v2) v1.cu = v2.cu; v1.kernel_id = get_kernel_id(v2.pc); return v1.raw; -}; +} +} // namespace namespace OccupancyFile { @@ -107,6 +110,5 @@ OccupancyFile(const Fspath& dir, OutputFile(dir / "occupancy.json") << jocc; } } // namespace OccupancyFile - } // namespace att_wrapper } // namespace rocprofiler diff --git a/source/lib/rocprofiler-sdk-att/outputfile.hpp b/source/lib/rocprofiler-sdk-att/outputfile.hpp index 3e8c5d4265..c433eca0bd 100644 --- a/source/lib/rocprofiler-sdk-att/outputfile.hpp +++ b/source/lib/rocprofiler-sdk-att/outputfile.hpp @@ -43,7 +43,7 @@ public: { if(!Enabled()) return; ofs = std::ofstream(str, std::ofstream::out); - ROCP_FATAL_IF(!ofs.is_open()) << "Could not open output file " << str << std::endl; + ROCP_FATAL_IF(!ofs.is_open()) << "Could not open output file " << str; } template diff --git a/source/lib/rocprofiler-sdk-att/profile_interface.cpp b/source/lib/rocprofiler-sdk-att/profile_interface.cpp index ae1f9eeae2..5687619a43 100644 --- a/source/lib/rocprofiler-sdk-att/profile_interface.cpp +++ b/source/lib/rocprofiler-sdk-att/profile_interface.cpp @@ -57,13 +57,13 @@ get_trace_data(rocprofiler_att_decoder_record_type_t trace_id, CHECK_NOTNULL(userdata); trace_data_t& trace_data = *reinterpret_cast(userdata); CHECK_NOTNULL(trace_data.tool); - ToolData& tool = *reinterpret_cast(trace_data.tool); + ToolData& tool = *trace_data.tool; if(trace_id == ROCPROFILER_ATT_DECODER_TYPE_INFO) { auto* infos = (rocprofiler_att_decoder_info_t*) trace_events; for(size_t i = 0; i < trace_size; i++) - ROCP_WARNING << tool.dl->att_info_fn(infos[i]) << std::endl; + ROCP_WARNING << tool.dl->att_info_fn(infos[i]); return ROCPROFILER_ATT_DECODER_STATUS_SUCCESS; } @@ -138,7 +138,7 @@ isa_callback(char* isa_instruction, CHECK_NOTNULL(userdata); trace_data_t& trace_data = *reinterpret_cast(userdata); CHECK_NOTNULL(trace_data.tool); - ToolData& tool = *reinterpret_cast(trace_data.tool); + ToolData& tool = *trace_data.tool; std::shared_ptr instruction{nullptr}; diff --git a/source/lib/rocprofiler-sdk-att/profile_interface.hpp b/source/lib/rocprofiler-sdk-att/profile_interface.hpp index dc71d30145..f67f21041d 100644 --- a/source/lib/rocprofiler-sdk-att/profile_interface.hpp +++ b/source/lib/rocprofiler-sdk-att/profile_interface.hpp @@ -47,9 +47,9 @@ } \ catch(std::exception & e) \ { \ - std::cerr << "Error in " << __FILE__ << ':' << __LINE__ << ' ' << e.what() << std::endl; \ + std::cerr << "Error in " << __FILE__ << ':' << __LINE__ << ' ' << e.what() << '\n'; \ } \ - catch(...) { std::cerr << "Error in " << __FILE__ << ':' << __LINE__ << std::endl; } + catch(...) { std::cerr << "Error in " << __FILE__ << ':' << __LINE__ << '\n'; } namespace rocprofiler { diff --git a/source/lib/rocprofiler-sdk-att/waitcnt/gfx10.cpp b/source/lib/rocprofiler-sdk-att/waitcnt/gfx10.cpp index 97a2821fc8..d57f1d7070 100644 --- a/source/lib/rocprofiler-sdk-att/waitcnt/gfx10.cpp +++ b/source/lib/rocprofiler-sdk-att/waitcnt/gfx10.cpp @@ -63,7 +63,9 @@ union MemoryInst static_assert(sizeof(MemoryInst) == sizeof(int)); -static MemoryInst +namespace +{ +MemoryInst classify(const std::string& inst) { constexpr size_t npos = std::string::npos; @@ -109,6 +111,7 @@ classify(const std::string& inst) else return MemoryInstType::TYPE_NOT_MEM; } +} // namespace std::vector WaitcntList::gfx10_construct(const wave_t& wave, isa_map_t& isa_map) diff --git a/source/lib/rocprofiler-sdk-att/waitcnt/gfx12.cpp b/source/lib/rocprofiler-sdk-att/waitcnt/gfx12.cpp index f167732ec8..14de766207 100644 --- a/source/lib/rocprofiler-sdk-att/waitcnt/gfx12.cpp +++ b/source/lib/rocprofiler-sdk-att/waitcnt/gfx12.cpp @@ -70,7 +70,9 @@ union MemoryInst static_assert(sizeof(MemoryInst) == sizeof(int)); -static MemoryInst +namespace +{ +MemoryInst classify(const std::string& inst) { constexpr size_t npos = std::string::npos; @@ -130,6 +132,7 @@ classify(const std::string& inst) else return MemoryInstType::TYPE_NOT_MEM; } +} // namespace std::vector WaitcntList::gfx12_construct(const wave_t& wave, isa_map_t& isa_map) diff --git a/source/lib/rocprofiler-sdk-att/wave.cpp b/source/lib/rocprofiler-sdk-att/wave.cpp index f20a143896..2c757fea41 100644 --- a/source/lib/rocprofiler-sdk-att/wave.cpp +++ b/source/lib/rocprofiler-sdk-att/wave.cpp @@ -62,10 +62,10 @@ WaveFile::WaveFile(WaveConfig& config, const att_wave_data_t& wave) for(size_t i = 0; i < wave.instructions_size; i++) { auto& inst = wave.instructions_array[i]; - instructions.push_back({(int64_t) inst.time, - (int) inst.category, - (int) inst.stall, - (int64_t) inst.duration, + instructions.push_back({inst.time, + static_cast(inst.category), + static_cast(inst.stall), + static_cast(inst.duration), config.code->line_numbers[inst.pc]}); } diff --git a/source/lib/rocprofiler-sdk-roctx/roctx.cpp b/source/lib/rocprofiler-sdk-roctx/roctx.cpp index 77a61c9493..5fb1f61e88 100644 --- a/source/lib/rocprofiler-sdk-roctx/roctx.cpp +++ b/source/lib/rocprofiler-sdk-roctx/roctx.cpp @@ -223,7 +223,7 @@ roctxRangeStartA(const char* message) void roctxRangeStop(roctx_range_id_t id) { - return ::rocprofiler::roctx::get_table()->core.roctxRangeStop_fn(id); + ::rocprofiler::roctx::get_table()->core.roctxRangeStop_fn(id); } int diff --git a/source/lib/rocprofiler-sdk/kernel_dispatch/profiling_time.cpp b/source/lib/rocprofiler-sdk/kernel_dispatch/profiling_time.cpp index f2d312e8ef..86a8c724ec 100644 --- a/source/lib/rocprofiler-sdk/kernel_dispatch/profiling_time.cpp +++ b/source/lib/rocprofiler-sdk/kernel_dispatch/profiling_time.cpp @@ -43,7 +43,7 @@ profiling_time get_dispatch_time(hsa_agent_t _hsa_agent, hsa_signal_t _signal, rocprofiler_kernel_id_t _kernel_id, - std::optional _baseline) + std::optional _baseline) // NOLINT(performance-unnecessary-value-param) { auto ts = common::timestamp_ns(); auto dispatch_time = hsa_amd_profiling_dispatch_time_t{}; diff --git a/source/lib/rocprofiler-sdk/page_migration/page_migration.cpp b/source/lib/rocprofiler-sdk/page_migration/page_migration.cpp index 530b212836..a78120f0f5 100644 --- a/source/lib/rocprofiler-sdk/page_migration/page_migration.cpp +++ b/source/lib/rocprofiler-sdk/page_migration/page_migration.cpp @@ -704,8 +704,8 @@ struct poll_kfd_t poll_kfd_t(const poll_kfd_t&) = delete; poll_kfd_t& operator=(const poll_kfd_t&) = delete; - poll_kfd_t(poll_kfd_t&&) = default; - poll_kfd_t& operator=(poll_kfd_t&&) = default; + poll_kfd_t(poll_kfd_t&&) noexcept = default; + poll_kfd_t& operator=(poll_kfd_t&&) noexcept = default; ~poll_kfd_t() {