Fixing Clang tidy errors (#195)

* Fixing Clang tidy errors * format-fix * Update code_object.hpp * Clang Tidy Fixes on the whole Source folder * Update source/CMakeLists.txt Co-authored-by: Madsen, Jonathan <Jonathan.Madsen@amd.com> * Addressing reviews * Correcting the logic for parsing att counters * Format Fix * Update source/lib/rocprofiler-sdk-att/tests/dummy_decoder.cpp Co-authored-by: Madsen, Jonathan <Jonathan.Madsen@amd.com> * Update source/lib/rocprofiler-sdk-att/tests/standalone_tool_main.cpp Co-authored-by: Madsen, Jonathan <Jonathan.Madsen@amd.com> * Update source/lib/rocprofiler-sdk-tool/config.cpp Co-authored-by: Madsen, Jonathan <Jonathan.Madsen@amd.com> * Formatting * Deactivate clang-tidy in source/lib/rocprofiler-sdk-att/tests --------- Co-authored-by: Ammar ELWazir <aelwazir@amd.com> Co-authored-by: Madsen, Jonathan <Jonathan.Madsen@amd.com> Co-authored-by: Jonathan R. Madsen <jonathanrmadsen@gmail.com> [ROCm/rocprofiler-sdk commit: 5410fabd3d]
2025-02-07 22:33:32 -06:00
@@ -5,6 +5,13 @@ if(ROCPROFILER_BUILD_CODECOV)
    set(CMAKE_BUILD_TYPE "Coverage")
 endif()

+#
+# by default, activate clang-tidy on all code in the source folder. unittest subfolders
+# can add `rocprofiler_deactivate_clang_tidy()` to their CMakeLists.txt to disable
+# clang-tidy linting
+#
+rocprofiler_activate_clang_tidy()
+
 add_subdirectory(include)
 add_subdirectory(lib)
 add_subdirectory(libexec)
@@ -125,7 +125,7 @@ write_json(json_output&         json_ar,
        auto att_filenames                  = tool_metadata.get_att_filenames();
        auto code_object_snapshot_filenames = std::vector<std::string>{};
        code_object_snapshot_filenames.reserve(code_object_load_info.size());
-        for(auto info : code_object_load_info)
+        for(const auto& info : code_object_load_info)
        {
            code_object_snapshot_filenames.emplace_back(fs::path(info.name).filename());
        }
@@ -240,9 +240,9 @@ std::vector<std::string>
 metadata::get_att_filenames() const
 {
    auto data = std::vector<std::string>{};
-    for(auto filenames : att_filenames)
+    for(const auto& filenames : att_filenames)
    {
-        for(auto file : filenames.second.second)
+        for(const auto& file : filenames.second.second)
        {
            data.emplace_back(fs::path(file).filename());
        }
@@ -74,7 +74,7 @@ ATTFileMgr::parseShader(int se_id, const std::vector<char>& data)
    WaveConfig config(se_id, filenames, codefile, wstates);
    ToolData   tooldata(data, config, dl);

-    if(config.occupancy.size()) occupancy.emplace(se_id, std::move(config.occupancy));
+    if(!config.occupancy.empty()) occupancy.emplace(se_id, std::move(config.occupancy));

    for(auto& [pc, kernel] : config.kernel_names)
        codefile->kernel_names.emplace(pc, std::move(kernel));
@@ -101,7 +101,7 @@ query_att_decode_capability()

    for(auto& [cap, libname] : get_lib_names())
    {
-        if(DL(libname).handle != 0) ret.push_back(cap);
+        if(DL(libname).handle != nullptr) ret.push_back(cap);
    }

    return ret;
@@ -134,7 +134,7 @@ ATTDecoder::parse(const Fspath&                       input_dir,

    ATTFileMgr mgr(output_dir, dl);

-    for(auto& file : codeobj_files)
+    for(const auto& file : codeobj_files)
    {
        if(file.name.find("memory://") == 0)
        {
@@ -151,7 +151,7 @@ ATTDecoder::parse(const Fspath&                       input_dir,
        }
    }

-    for(auto& shader : att_files)
+    for(const auto& shader : att_files)
    {
        int shader_id = 0;
        try
@@ -186,7 +186,8 @@ ATTDecoder::parse(const Fspath&                       input_dir,
 bool
 ATTDecoder::valid() const
 {
-    return dl && dl->att_parse_data_fn && dl->att_info_fn && dl->att_status_fn;
+    return dl && (dl->att_parse_data_fn != nullptr) && (dl->att_info_fn != nullptr) &&
+           (dl->att_status_fn != nullptr);
 }

 }  // namespace att_wrapper
@@ -48,7 +48,7 @@ navigate(nlohmann::json& json, std::vector<std::string>& path, const std::string
    navigate(j, path, filename);
 }

-CodeFile::CodeFile(const Fspath& _dir, std::shared_ptr<AddressTable> _table)
+CodeFile::CodeFile(const Fspath& _dir, std::shared_ptr<AddressTable>& _table)
 : dir(_dir)
 , filename(_dir / "code.json")
 , table(_table)
@@ -145,7 +145,7 @@ CodeFile::~CodeFile()
    nlohmann::json jsnapfiletree;
    size_t         num_snap = 0;

-    for(auto& source_ref : snapshots)
+    for(const auto& source_ref : snapshots)
    {
        if(rocprofiler::common::filesystem::exists(source_ref))
        {
@@ -52,7 +52,8 @@ class CodeFile
    using AddressTable = rocprofiler::sdk::codeobj::disassembly::CodeobjAddressTranslate;

 public:
-    CodeFile(const Fspath& dir, std::shared_ptr<AddressTable> table);
+    CodeFile() = default;
+    CodeFile(const Fspath& dir, std::shared_ptr<AddressTable>& table);
    ~CodeFile();

    Fspath                                        dir{};
@@ -43,7 +43,7 @@ namespace att_wrapper
 namespace fs = ::rocprofiler::common::filesystem;

 fs::path
-get_search_path(std::string path_name)
+get_search_path(std::string_view path_name)
 {
    if(fs::exists(path_name)) return fs::path(path_name);
    return "";
@@ -73,18 +73,20 @@ convert(const att_occupancy_info_v2_t& v2)
    return v1.raw;
 };

+namespace OccupancyFile
+{
 void
-OccupancyFile::OccupancyFile(const Fspath&                                                 dir,
-                             std::shared_ptr<AddressTable>                                 table,
-                             const std::map<size_t, std::vector<att_occupancy_info_v2_t>>& occ)
+OccupancyFile(const Fspath&                                                 dir,
+              std::shared_ptr<AddressTable>&                                table,
+              const std::map<size_t, std::vector<att_occupancy_info_v2_t>>& occ)
 {
    if(!GlobalDefs::get().has_format("json")) return;
    nlohmann::json jocc;

-    for(auto& [se, eventlist] : occ)
+    for(const auto& [se, eventlist] : occ)
    {
        nlohmann::json list;
-        for(auto& event : eventlist)
+        for(const auto& event : eventlist)
            list.push_back(convert(event));
        jocc[std::to_string(se)] = list;
    }
@@ -104,6 +106,7 @@ OccupancyFile::OccupancyFile(const Fspath&

    OutputFile(dir / "occupancy.json") << jocc;
 }
+}  // namespace OccupancyFile

 }  // namespace att_wrapper
 }  // namespace rocprofiler
@@ -38,7 +38,7 @@ using AddressTable = rocprofiler::sdk::codeobj::disassembly::CodeobjAddressTrans

 void
 OccupancyFile(const Fspath&                                                 dir,
-              std::shared_ptr<AddressTable>                                 table,
+              std::shared_ptr<AddressTable>&                                table,
              const std::map<size_t, std::vector<att_occupancy_info_v2_t>>& occ);
 };  // namespace OccupancyFile

@@ -183,13 +183,13 @@ ToolData::ToolData(const std::vector<char>& _data, WaveConfig& _config, std::sha
        ROCP_ERROR << "Callback failed with status " << dl->att_status_fn(status);
 }

-ToolData::~ToolData() {}
+ToolData::~ToolData() = default;

 std::string
 demangle(std::string_view line)
 {
    int   status;
-    char* c_name = abi::__cxa_demangle(line.data(), 0, 0, &status);
+    char* c_name = abi::__cxa_demangle(line.data(), nullptr, nullptr, &status);

    if(c_name == nullptr) return "";

@@ -205,7 +205,7 @@ ToolData::get(pcinfo_t _pc)
    if(isa_map.find(_pc) != isa_map.end()) return *isa_map.at(_pc);

    // Attempt to disassemble full kernel
-    if(_pc.marker_id) try
+    if(_pc.marker_id != 0u) try
        {
            rocprofiler::sdk::codeobj::segment::CodeobjTableTranslator symbol_table;
            for(auto& [vaddr, symbol] : cfile->table->getSymbolMap(_pc.marker_id))
@@ -232,7 +232,7 @@ ToolData::get(pcinfo_t _pc)

                cline.code_line = cfile->table->get(addr_range.id, addr);
                addr += cline.code_line->size;
-                if(!cline.code_line->size) throw std::invalid_argument("Line has 0 bytes!");
+                if(cline.code_line->size == 0u) throw std::invalid_argument("Line has 0 bytes!");
            }

            if(isa_map.find(_pc) != isa_map.end()) return *isa_map.at(_pc);
@@ -3,6 +3,8 @@
 #
 project(rocprofiler-att-parser-tests LANGUAGES CXX)

+rocprofiler_deactivate_clang_tidy()
+
 add_executable(att-parser-tool-v3)
 target_link_libraries(
    att-parser-tool-v3
@@ -102,7 +102,7 @@ TEST(att_decoder_test, warn_failures)
    codeobjs.at(4).name = "myfile123.out";

    std::vector<std::string> att_files;
-    att_files.emplace_back(std::string("file123.att"));
+    att_files.emplace_back("file123.att");

    ATTDecoderTest decoder;
    ROCP_FATAL_IF(!decoder.valid()) << "Failed to initialize decoder library!";
@@ -116,7 +116,7 @@ TEST(att_decoder_test, code_write)
    rocprofiler::att_wrapper::OutputFile::Enabled() = false;
    GlobalDefs::get().output_formats                = "json,csv";

-    CodeFile file(".", nullptr);
+    CodeFile file{};

    pcinfo_t addr{};
    addr.marker_id          = 0;
@@ -33,7 +33,11 @@ rocprofiler_att_decoder_parse_data(rocprofiler_att_decoder_se_data_callback_t se
 {
    const int gfxip = 9;

-    trace_callback(ROCPROFILER_ATT_DECODER_TYPE_GFXIP, 0, (void*) gfxip, 0, userdata);
+    trace_callback(ROCPROFILER_ATT_DECODER_TYPE_GFXIP,
+                   0,
+                   reinterpret_cast<void*>(gfxip),  // NOLINT(performance-no-int-to-ptr)
+                   0,
+                   userdata);
    {
        std::vector<rocprofiler_att_decoder_info_t> infos{};
        for(size_t i = 1; i < ROCPROFILER_ATT_DECODER_INFO_LAST; i++)
@@ -52,7 +56,7 @@ rocprofiler_att_decoder_parse_data(rocprofiler_att_decoder_se_data_callback_t se
        uint8_t* buffer      = nullptr;
        size_t   buffer_size = 0;

-        while(se_data_callback(&se_id, &buffer, &buffer_size, userdata))
+        while(se_data_callback(&se_id, &buffer, &buffer_size, userdata) != 0u)
        {};
    }

@@ -101,7 +105,7 @@ rocprofiler_att_decoder_parse_data(rocprofiler_att_decoder_se_data_callback_t se
            inst.time         = i * 64 - 32;
            inst.pc.marker_id = 1;
            inst.pc.addr      = 8 * i;
-            insts.emplace_back(std::move(inst));
+            insts.emplace_back(inst);
        }

        wave.instructions_array = insts.data();
@@ -123,7 +123,7 @@ main(int argc, char** argv)
            snapshot_files[elem["key"]] = elem["value"];

        for(auto& codeobj : sdk_json["code_objects"])
-            if(std::string(codeobj["uri"]).size())
+            if(!std::string{codeobj["uri"]}.empty())
            {
                std::string filename = codeobj["uri"];
                size_t      id       = size_t(codeobj["code_object_id"]);
@@ -87,7 +87,7 @@ public:
        MEMORY_PARALLEL
    };

-    MemoryCounter(std::string _name)
+    MemoryCounter(std::string_view _name)
    : name(_name)
    {}

@@ -129,13 +129,13 @@ WaitcntList::gfx9_construct(const wave_t& wave, isa_map_t& isa_map)
        }
        else if(typeclass == MemoryInstType::TYPE_WAITCNT)
        {
-            if(type & LGK_BIT)
+            if((type & LGK_BIT) != 0)
            {
                if(auto joined = lgkm.handle_mem_op(inst_str, vflat_list))
                    mem_unroll.emplace_back(LineWaitcnt{line_number, std::move(*joined)});
            }

-            if(type & VMEM_BIT)
+            if((type & VMEM_BIT) != 0)
            {
                if(auto joined = vmem.handle_mem_op(inst_str, vflat_list))
                    mem_unroll.emplace_back(LineWaitcnt{line_number, std::move(*joined)});
@@ -93,14 +93,14 @@ TEST(att_decoder_waitcnt_test, gfx9)
    wave.instructions_array = insts.data();
    wave.instructions_size  = insts.size();

-    auto& data = WaitcntList::Get(9, wave, isa_map);
+    const auto& data = WaitcntList::Get(9, wave, isa_map);

    std::map<int, std::set<int>> dependencies{};

-    for(auto& [dst, src] : data.mem_unroll)
+    for(const auto& [dst, src] : data.mem_unroll)
    {
        auto& dep = dependencies[dst];
-        for(auto& p : src)
+        for(const auto& p : src)
            dep.insert(p);
    }

@@ -183,14 +183,14 @@ TEST(att_decoder_waitcnt_test, gfx10)
    wave.instructions_array = insts.data();
    wave.instructions_size  = insts.size();

-    auto& data = WaitcntList::Get(10, wave, isa_map);
+    const auto& data = WaitcntList::Get(10, wave, isa_map);

    std::map<int, std::set<int>> dependencies{};

-    for(auto& [dst, src] : data.mem_unroll)
+    for(const auto& [dst, src] : data.mem_unroll)
    {
        auto& dep = dependencies[dst];
-        for(auto& p : src)
+        for(const auto& p : src)
            dep.insert(p);
    }

@@ -300,14 +300,14 @@ TEST(att_decoder_waitcnt_test, gfx12)
    wave.instructions_array = insts.data();
    wave.instructions_size  = insts.size();

-    auto& data = WaitcntList::Get(12, wave, isa_map);
+    const auto& data = WaitcntList::Get(12, wave, isa_map);

    std::map<int, std::set<int>> dependencies{};

-    for(auto& [dst, src] : data.mem_unroll)
+    for(const auto& [dst, src] : data.mem_unroll)
    {
        auto& dep = dependencies[dst];
-        for(auto& p : src)
+        for(const auto& p : src)
            dep.insert(p);
    }

@@ -34,7 +34,8 @@ namespace att_wrapper
 {
 WaveFile::WaveFile(WaveConfig& config, const att_wave_data_t& wave)
 {
-    if(wave.contexts) ROCP_WARNING << "Wave had " << wave.contexts << " context save-restores";
+    ROCP_WARNING_IF(wave.contexts != 0u)
+        << "Wave had " << wave.contexts << " context save-restores";

    if(!GlobalDefs::get().has_format("json")) return;
    if(wave.instructions_size == 0 && wave.timeline_size < 3) return;
@@ -89,8 +90,8 @@ WaveFile::WaveFile(WaveConfig& config, const att_wave_data_t& wave)
        const WaitcntList& wait_list =
            WaitcntList::Get(config.filemgr->gfxip, wave, config.code->isa_map);

-        for(auto& line : wait_list.mem_unroll)
-            if(line.dependencies.size())
+        for(const auto& line : wait_list.mem_unroll)
+            if(!line.dependencies.empty())
            {
                nlohmann::json json_line;
                for(int dep : line.dependencies)
@@ -38,7 +38,7 @@ WstatesFile::WstatesFile(int state, const Fspath& dir)

 WstatesFile::~WstatesFile()
 {
-    if(!events.size() || !GlobalDefs::get().has_format("json")) return;
+    if(events.empty() || !GlobalDefs::get().has_format("json")) return;

    std::sort(events.begin(), events.end(), [](const event_t& a, const event_t& b) {
        return a.first < b.first;
@@ -52,7 +52,7 @@ WstatesFile::~WstatesFile()
    for(auto& [time, value] : events)
    {
        accum += value;
-        if(!jtime.size() || time != prev_time)
+        if(jtime.empty() || time != prev_time)
        {
            jtime.push_back(time);
            jstate.push_back(accum);
@@ -163,8 +163,7 @@ parse_att_counters(std::string line)
    handle_special_chars(line);

    auto extract_counter_name_and_simd_mask = [](std::string& input) {
-        std::string counter_name = "";
-        auto        ret          = att_perfcounter{};
+        auto ret = att_perfcounter{};

        size_t pos = input.find(':');

@@ -174,7 +173,9 @@ parse_att_counters(std::string line)
            ret.simd_mask    = std::stoi(input.substr(pos + 1), nullptr, 16);
        }
        else
-            counter_name = input;
+        {
+            ret.counter_name = input;
+        }
        return ret;
    };

@@ -1394,7 +1394,8 @@ tool_init(rocprofiler_client_finalize_t fini_func, void* tool_data)
        parameters.push_back({ROCPROFILER_ATT_PARAMETER_SIMD_SELECT, {simd_select}});
        parameters.push_back({ROCPROFILER_ATT_PARAMETER_BUFFER_SIZE, {buffer_sz}});
        parameters.push_back({ROCPROFILER_ATT_PARAMETER_SHADER_ENGINE_MASK, {shader_mask}});
-        parameters.push_back({ROCPROFILER_ATT_PARAMETER_SERIALIZE_ALL, {att_serialize_all}});
+        parameters.push_back(
+            {ROCPROFILER_ATT_PARAMETER_SERIALIZE_ALL, {static_cast<uint64_t>(att_serialize_all)}});

        ROCPROFILER_CALL(
            rocprofiler_configure_dispatch_thread_trace_service(get_client_ctx(),
@@ -30,7 +30,6 @@
 #include "lib/common/utility.hpp"
 #include "lib/rocprofiler-sdk/agent.hpp"
 #include "lib/rocprofiler-sdk/code_object/code_object.hpp"
-#include "lib/rocprofiler-sdk/code_object/hip/code_object.hpp"

 #include <rocprofiler-sdk/fwd.h>
 #include <rocprofiler-sdk/hsa.h>