Miscellaneous updates (#959)

- missing-new-line CI job: ensures all source files end with new line
- logging updates
- add new line to the end of many files
- fix header include ordering is misc places
- transition to use hsa::get_core_table() and hsa::get_amd_ext_table() in various places instead of making copies

.github/workflows/formatting.yml

@@ -143,3 +143,21 @@ jobs:
         command: review
         pull_number: ${{ github.event.pull_request.number }}
         git_dir: '.'
+
+  missing-new-line:
+    runs-on: ubuntu-22.04
+
+    steps:
+    - uses: actions/checkout@v4
+
+    - name: Find missing new line
+      shell: bash
+      run: |
+        OUTFILE=missing_newline.txt
+        for i in $(find source/lib source/include tests samples cmake -type f | egrep -v '\.bin$'); do VAL=$(tail -c 1 ${i}); if [ -n "${VAL}" ]; then echo "- ${i}" >> ${OUTFILE}; fi; done
+        if [[ -f ${OUTFILE} && $(cat ${OUTFILE} | wc -l) -gt 0 ]]; then
+          echo -e "\nError! Source code missing new line at end of file...\n"
+          echo -e "\nFiles:\n"
+          cat ${OUTFILE}
+          exit 1
+        fi

samples/pc_sampling/pcs.cpp

@@ -160,11 +160,12 @@ query_avail_configs_for_agent(tool_agent_info* agent_info)
     {
         // The query operation failed, so consider the PC sampling is unsupported at the agent.
         // This can happen if the PC sampling service is invoked within the ROCgdb.
-        ss << "Querying PC sampling capabilities failed with status: " << status << std::endl;
+        ss << "Querying PC sampling capabilities failed with status=" << status
+           << " :: " << rocprofiler_get_status_string(status) << std::endl;
         *utils::get_output_stream() << ss.str() << std::endl;
         return false;
     }
-    else if(agent_info->avail_configs->size() == 0)
+    else if(agent_info->avail_configs->empty())
     {
         // No available configuration at the moment, so mark the PC sampling as unsupported.
         return false;

source/lib/common/utility.cpp

@@ -148,4 +148,4 @@ rocprofiler_debugger_continue()
 {
     debugger_block().exchange(false);
 }
-}
+}

source/lib/common/utility.hpp

@@ -271,4 +271,4 @@ void
 rocprofiler_debugger_block();
 void
 rocprofiler_debugger_continue();
-}
+}

source/lib/rocprofiler-sdk-codeobj/tests/CMakeLists.txt

@@ -29,5 +29,5 @@ set_tests_properties(${codeobj-library-test_TESTS} PROPERTIES TIMEOUT 10 LABELS
 target_compile_definitions(codeobj-library-test
                            PRIVATE -DCODEOBJ_BINARY_DIR=\"${CMAKE_CURRENT_BINARY_DIR}/\")
 
-configure_file(smallkernel.b smallkernel.b COPYONLY)
+configure_file(smallkernel.bin smallkernel.bin COPYONLY)
 configure_file(hipcc_output.s hipcc_output.s COPYONLY)

source/lib/rocprofiler-sdk-codeobj/tests/codeobj_library_test.cpp

@@ -69,7 +69,7 @@ static const std::vector<char>&
 GetCodeobjContents()
 {
     static std::vector<char> buffer = []() {
-        std::string   filename = CODEOBJ_BINARY_DIR "smallkernel.b";
+        std::string   filename = CODEOBJ_BINARY_DIR "smallkernel.bin";
         std::ifstream file(filename.data(), std::ios::binary);
 
         using iterator_t = std::istreambuf_iterator<char>;
@@ -141,7 +141,7 @@ TEST(codeobj_library, decoder_component)
 
     CodeobjDecoderComponent component(objdata.data(), objdata.size());
 
-    std::string          kernel_with_protocol = "file://" CODEOBJ_BINARY_DIR "smallkernel.b";
+    std::string          kernel_with_protocol = "file://" CODEOBJ_BINARY_DIR "smallkernel.bin";
     LoadedCodeobjDecoder loadecomp(kernel_with_protocol.data(), loaded_offset, objdata.size());
 
     ASSERT_EQ(component.m_symbol_map.size(), 1);

source/lib/rocprofiler-sdk/agent_profile.cpp

@@ -47,4 +47,4 @@ rocprofiler_sample_agent_profile_counting_service(rocprofiler_context_id_t   con
     return rocprofiler::counters::read_agent_ctx(
         rocprofiler::context::get_registered_context(context_id), user_data, flags);
 }
-}
+}

source/lib/rocprofiler-sdk/aql/helpers.cpp

@@ -21,15 +21,15 @@
 // SOFTWARE.
 
 #include "lib/rocprofiler-sdk/aql/helpers.hpp"
-
-#include <fmt/core.h>
-
-#include <rocprofiler-sdk/fwd.h>
-
 #include "lib/common/logging.hpp"
 #include "lib/common/synchronized.hpp"
 #include "lib/common/utility.hpp"
 #include "lib/rocprofiler-sdk/counters/id_decode.hpp"
+#include "lib/rocprofiler-sdk/hsa/hsa.hpp"
+
+#include <rocprofiler-sdk/fwd.h>
+
+#include <fmt/core.h>
 
 namespace rocprofiler
 {
@@ -114,11 +114,15 @@ get_dim_info(rocprofiler_agent_id_t   agent,
 }
 
 rocprofiler_status_t
-set_profiler_active_on_queue(const AmdExtTable&                api,
-                             hsa_amd_memory_pool_t             pool,
+set_profiler_active_on_queue(hsa_amd_memory_pool_t             pool,
                              hsa_agent_t                       hsa_agent,
                              const rocprofiler_profile_pkt_cb& packet_submit)
 {
+    CHECK(hsa::get_amd_ext_table() != nullptr);
+    CHECK(hsa::get_amd_ext_table()->hsa_amd_memory_pool_allocate_fn != nullptr);
+    CHECK(hsa::get_amd_ext_table()->hsa_amd_agents_allow_access_fn != nullptr);
+    CHECK(hsa::get_amd_ext_table()->hsa_amd_memory_pool_free_fn != nullptr);
+
     // Inject packet to enable profiling of other process queues on this queue
     hsa_ven_amd_aqlprofile_profile_t profile{};
     profile.agent = hsa_agent;
@@ -134,15 +138,15 @@ set_profiler_active_on_queue(const AmdExtTable&                api,
     const size_t mask = 0x1000 - 1;
     auto         size = (profile.command_buffer.size + mask) & ~mask;
 
-    if(api.hsa_amd_memory_pool_allocate_fn(pool, size, 0, &profile.command_buffer.ptr) !=
-       HSA_STATUS_SUCCESS)
+    if(hsa::get_amd_ext_table()->hsa_amd_memory_pool_allocate_fn(
+           pool, size, 0, &profile.command_buffer.ptr) != HSA_STATUS_SUCCESS)
     {
         ROCP_WARNING << "Failed to allocate memory to enable profile command on agent, some "
                         "counters will be unavailable";
         return ROCPROFILER_STATUS_ERROR;
     }
-    if(api.hsa_amd_agents_allow_access_fn(1, &hsa_agent, nullptr, profile.command_buffer.ptr) !=
-       HSA_STATUS_SUCCESS)
+    if(hsa::get_amd_ext_table()->hsa_amd_agents_allow_access_fn(
+           1, &hsa_agent, nullptr, profile.command_buffer.ptr) != HSA_STATUS_SUCCESS)
     {
         ROCP_WARNING << "Agent cannot access memory, some counters will be unavailable";
         return ROCPROFILER_STATUS_ERROR;
@@ -157,7 +161,7 @@ set_profiler_active_on_queue(const AmdExtTable&                api,
     }
 
     packet_submit(packet);
-    api.hsa_amd_memory_pool_free_fn(profile.command_buffer.ptr);
+    hsa::get_amd_ext_table()->hsa_amd_memory_pool_free_fn(profile.command_buffer.ptr);
     return ROCPROFILER_STATUS_SUCCESS;
 }
 

source/lib/rocprofiler-sdk/aql/helpers.hpp

@@ -22,18 +22,18 @@
 
 #pragma once
 
-#include <functional>
-#include <map>
-#include <string>
-
-#include <hsa/hsa_ven_amd_aqlprofile.h>
-
-#include <rocprofiler-sdk/fwd.h>
-
 #include "lib/rocprofiler-sdk/agent.hpp"
 #include "lib/rocprofiler-sdk/counters/metrics.hpp"
 #include "lib/rocprofiler-sdk/hsa/rocprofiler_packet.hpp"
 
+#include <rocprofiler-sdk/fwd.h>
+
+#include <hsa/hsa_ven_amd_aqlprofile.h>
+
+#include <functional>
+#include <map>
+#include <string>
+
 namespace rocprofiler
 {
 namespace aql
@@ -62,8 +62,7 @@ set_dim_id_from_sample(rocprofiler_counter_instance_id_t& id,
                        size_t                             sample_id);
 
 rocprofiler_status_t
-set_profiler_active_on_queue(const AmdExtTable&                api,
-                             hsa_amd_memory_pool_t             pool,
+set_profiler_active_on_queue(hsa_amd_memory_pool_t             pool,
                              hsa_agent_t                       hsa_agent,
                              const rocprofiler_profile_pkt_cb& packet_submit);
 }  // namespace aql

source/lib/rocprofiler-sdk/aql/tests/aql_test.cpp

@@ -219,4 +219,4 @@ TEST(aql_profile, test_aql_packet)
     // Why is this valid?
     TestAqlPacket test_pkt2(false);
 }
-*/
+*/