From 513ef996b386c5bb7c926a7d42ba6bab86e77cab Mon Sep 17 00:00:00 2001 From: John Cortell Date: Wed, 15 Mar 2023 11:47:35 -0500 Subject: [PATCH 1/2] The VkColorCube app doesn't log failure status The failure status code being returned via the GPA calls aren't being logged by the VkColorCube app. It does log that what it's trying to do failed, but not why it failed (according to the GPA implementation). In my case a C++ exception was being thrown and that fact was just not surfaced at all. Also changed the textual string for kGpaStatusErrorException to mention it's specifically a C++ exception, as otherwise the word "exception" is generic/vague and could be interepreted as such. --- .../vulkan/vk_color_cube/vk_color_cube.cc | 46 ++++++++++++------- .../vulkan/vk_color_cube/vk_color_cube.h | 19 ++++---- source/gpu_perf_api_common/gpu_perf_api.cc | 15 +++++- 3 files changed, 52 insertions(+), 28 deletions(-) diff --git a/source/examples/vulkan/vk_color_cube/vk_color_cube.cc b/source/examples/vulkan/vk_color_cube/vk_color_cube.cc index 92d691bd..090ae242 100644 --- a/source/examples/vulkan/vk_color_cube/vk_color_cube.cc +++ b/source/examples/vulkan/vk_color_cube/vk_color_cube.cc @@ -372,7 +372,7 @@ bool AMDVulkanDemo::InitializeGpa() gpu_perf_api_helper_.gpa_function_table_->GpaRegisterLoggingCallback(gpa_log_types, gpu_perf_api_helper_.gpaLoggingCallback); if (status_register_callback != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to register GPA logging callback."); + LogStatus(status_register_callback, "ERROR: Failed to register GPA logging callback"); return false; } @@ -380,7 +380,7 @@ bool AMDVulkanDemo::InitializeGpa() if (status_gpa_initialize != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to initialize GPA."); + LogStatus(status_gpa_initialize, "ERROR: Failed to initialize GPA"); return false; } @@ -421,7 +421,6 @@ bool AMDVulkanDemo::InitializeGpa() if (kGpaStatusOk != status) { AMDVulkanDemoVkUtils::Log("ERROR: GpaGetFuncTable failed with status %d", status); - delete gpa_function_table; return false; } @@ -440,7 +439,7 @@ bool AMDVulkanDemo::InitializeGpa() status = gpu_perf_api_helper_.gpa_function_table_->GpaRegisterLoggingCallback(gpa_log_types, gpu_perf_api_helper_.gpaLoggingCallback); if (status != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to register GPA logging callback."); + LogStatus(status, "ERROR: Failed to register GPA logging callback"); return false; } @@ -864,7 +863,7 @@ bool AMDVulkanDemo::InitializeVulkan() if (gpa_open_context_status != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to open GPA context."); + LogStatus(gpa_open_context_status, "ERROR: Failed to open GPA context"); return false; } @@ -878,7 +877,7 @@ bool AMDVulkanDemo::InitializeVulkan() GpaStatus get_sample_types_status = gpu_perf_api_helper_.gpa_function_table_->GpaGetSupportedSampleTypes(gpa_context_id_, &sample_types); if (get_sample_types_status != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to get supported GPA sample types."); + LogStatus(get_sample_types_status, "ERROR: Failed to get supported GPA sample types"); return false; } @@ -887,7 +886,7 @@ bool AMDVulkanDemo::InitializeVulkan() if (gpa_create_session_status != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to create GPA session."); + LogStatus(gpa_create_session_status, "ERROR: Failed to create GPA session"); return false; } @@ -914,6 +913,7 @@ bool AMDVulkanDemo::InitializeVulkan() if (gpa_status != kGpaStatusOk) { AMDVulkanDemoVkUtils::Log("Failed to enable counter: %s", it->c_str()); + LogStatus(gpa_status); } success_enable_counter &= gpa_status == kGpaStatusOk; } @@ -941,7 +941,7 @@ bool AMDVulkanDemo::InitializeVulkan() if (gpa_enable_all_counters_status != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to enable all GPA counters."); + LogStatus(gpa_enable_all_counters_status, "ERROR: Failed to enable all GPA counters"); return false; } @@ -950,7 +950,7 @@ bool AMDVulkanDemo::InitializeVulkan() GpaStatus gpa_get_pass_count_status = gpu_perf_api_helper_.gpa_function_table_->GpaGetPassCount(gpa_session_id_, &required_pass_count_); if (gpa_get_pass_count_status != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to get the number of required GPA passes."); + LogStatus(gpa_get_pass_count_status, "ERROR: Failed to get the number of required GPA passes"); return false; } @@ -965,7 +965,7 @@ bool AMDVulkanDemo::InitializeVulkan() if (gpa_begin_session_status != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to begin GPA session."); + LogStatus(gpa_begin_session_status, "ERROR: Failed to begin GPA session"); return false; } } @@ -1654,7 +1654,7 @@ bool AMDVulkanDemo::InitializeVulkan() if (gpa_end_session_status != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to end GPA session."); + LogStatus(gpa_end_session_status, "ERROR: Failed to end GPA session"); } } @@ -1814,9 +1814,10 @@ void AMDVulkanDemo::DrawScene() // This example only renders one set of profiles (aka, only the number of passes needed to generate one set of results). unsigned int profile_set = 0; - gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, AMDVulkanDemo::kGpaSampleIdCube, print_debug_output_, Verify(), ConfirmSuccess()); - gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, AMDVulkanDemo::kGpaSampleIdWireframe, print_debug_output_, Verify(), ConfirmSuccess()); - gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, AMDVulkanDemo::kGpaSampleIdCubeAndWireframe, print_debug_output_, Verify(), ConfirmSuccess()); + // NOTE: we can't loop over these because it is not guaranteed that the sample_ids will be 0-based and monotonically increasing. + gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, 0, print_debug_output_, Verify(), ConfirmSuccess()); + gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, 1, print_debug_output_, Verify(), ConfirmSuccess()); + gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, 2, print_debug_output_, Verify(), ConfirmSuccess()); // Close the CSV file so that it actually gets saved out. gpu_perf_api_helper_.CloseCSVFile(); @@ -1842,7 +1843,7 @@ void AMDVulkanDemo::Destroy() if (gpa_delete_session_status != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to delete GPA session."); + LogStatus(gpa_delete_session_status, "ERROR: Failed to delete GPA session"); } } @@ -1852,7 +1853,7 @@ void AMDVulkanDemo::Destroy() if (gpa_close_context_status != kGpaStatusOk) { - AMDVulkanDemoVkUtils::Log("ERROR: Failed to close GPA Context."); + LogStatus(gpa_close_context_status, "ERROR: Failed to close GPA Context"); } } } @@ -2358,6 +2359,19 @@ void AMDVulkanDemo::PreBuildCommandBuffers(PrebuiltPerFrameResources* prebuilt_r } } +void AMDVulkanDemo::LogStatus(GpaStatus status, const char* msg) +{ + auto status_as_str = gpu_perf_api_helper_.gpa_function_table_->GpaGetStatusAsStr(status); + if (msg != nullptr) + { + AMDVulkanDemoVkUtils::Log("%s. %s", msg, status_as_str); + } + else + { + AMDVulkanDemoVkUtils::Log("%s", status_as_str); + } +} + #if defined(VK_USE_PLATFORM_WIN32_KHR) /// Window message processing callback. diff --git a/source/examples/vulkan/vk_color_cube/vk_color_cube.h b/source/examples/vulkan/vk_color_cube/vk_color_cube.h index 78cb0ea3..0fe8476e 100644 --- a/source/examples/vulkan/vk_color_cube/vk_color_cube.h +++ b/source/examples/vulkan/vk_color_cube/vk_color_cube.h @@ -142,13 +142,6 @@ class AMDVulkanDemo : public gpa_example::GpaSampleApp /// Default window height. const uint32_t kDefaultWindowHeight = 300; - /// Note that GPA sample IDs are client defined. However, the Vulkan GPA - /// extension assigns an ID to each sample (they are not client defined). - /// The GPA library manages the mapping between them. These are the former. - static constexpr GpaUInt32 kGpaSampleIdCube = 0; - static constexpr GpaUInt32 kGpaSampleIdWireframe = 1; - static constexpr GpaUInt32 kGpaSampleIdCubeAndWireframe = 2; - #ifdef ANDROID inline void SetWindow(ANativeWindow* native_window) { @@ -249,7 +242,7 @@ class AMDVulkanDemo : public gpa_example::GpaSampleApp /// Sample Id that the application (not GPA) assigns to the cube. /// The cube will have this same sample_id in all passes. - const GpaUInt32 gpa_sample_id = kGpaSampleIdCube; + const GpaUInt32 gpa_sample_id = 0; } cube_; /// @brief Container for objects related to drawing the wireframe. @@ -263,7 +256,7 @@ class AMDVulkanDemo : public gpa_example::GpaSampleApp /// Sample Id that the application (not GPA) assigns to the wireframe. /// The wireframe will have this same sample_id in all passes. - const GpaUInt32 gpa_sample_id = kGpaSampleIdWireframe; + const GpaUInt32 gpa_sample_id = 1; } wire_frame_; /// @brief Container for objects related to drawing the cube and wireframe. @@ -286,7 +279,7 @@ class AMDVulkanDemo : public gpa_example::GpaSampleApp /// Sample Id that the application (not GPA) assigns to the cube wireframe. /// The combined cube + wireframe sample will have this same sample_id in all passes. - const GpaUInt32 gpa_sample_id = kGpaSampleIdCubeAndWireframe; + const GpaUInt32 gpa_sample_id = 2; } cube_and_wire_frame_; }; @@ -394,6 +387,12 @@ class AMDVulkanDemo : public gpa_example::GpaSampleApp /// @param [in] gpa_pass_index If GPA is enabled for these command buffers, this indicates which profile pass is being built; ignored if enable_gpa is false. void PreBuildCommandBuffers(PrebuiltPerFrameResources* prebuilt_resources, VkFramebuffer frame_buffer, bool enable_gpa, uint32_t gpa_pass_index); + /// @brief Log the textual representation of a failure status code + /// + /// @param [in] status the failure code + /// @param [in] msg optional additional context. Should not contain trailing punctuation. + void LogStatus(GpaStatus status, const char* msg=nullptr); + /// GPA helper. GpaHelper gpu_perf_api_helper_; diff --git a/source/gpu_perf_api_common/gpu_perf_api.cc b/source/gpu_perf_api_common/gpu_perf_api.cc index 76458ffd..71c164be 100644 --- a/source/gpu_perf_api_common/gpu_perf_api.cc +++ b/source/gpu_perf_api_common/gpu_perf_api.cc @@ -47,7 +47,7 @@ extern IGpaImplementor* gpa_imp; ///< GPA implementor instance. } \ if (index >= num_counters) \ { \ - GPA_LOG_ERROR("Parameter %s is %d but must be less than %d.", #index, index, num_counters); \ + GPA_LOG_ERROR("Parameter index is %d but must be less than %d.", index, num_counters); \ return kGpaStatusErrorIndexOutOfRange; \ } @@ -493,6 +493,15 @@ GPA_LIB_DECL GpaStatus GpaGetDeviceGeneration(GpaContextId gpa_context_id, GpaHw case GDT_HW_GENERATION_GFX11: *hardware_generation = kGpaHwGenerationGfx11; break; + case GDT_HW_GENERATION_GFX104: + *hardware_generation = kGpaHwGenerationGfx104; + break; + case GDT_HW_GENERATION_GFX401: + *hardware_generation = kGpaHwGenerationGfx401; + break; + case GDT_HW_GENERATION_GFX402: + *hardware_generation = kGpaHwGenerationGfx402; + break; case GDT_HW_GENERATION_LAST: *hardware_generation = kGpaHwGenerationLast; break; @@ -847,6 +856,8 @@ GPA_LIB_DECL GpaStatus GpaDeleteSession(GpaSessionId gpa_session_id) GPA_LIB_DECL GpaStatus GpaBeginSession(GpaSessionId gpa_session_id) { + GPA_LOG_ERROR("jjjjjjjjjjjjj GpaBeginSession"); + try { PROFILE_FUNCTION(GpaBeginSession); @@ -1657,7 +1668,7 @@ static const char* kErrorString[] = { GPA_ENUM_STRING_VAL(kGpaStatusErrorTimeout, "GPA Error: Attempt to Retrieve Data Failed due to Timeout."), GPA_ENUM_STRING_VAL(kGpaStatusErrorLibAlreadyLoaded, "GPA Error: Library Is Already Loaded."), GPA_ENUM_STRING_VAL(kGpaStatusErrorOtherSessionActive, "GPA Error: Other Session Is Active."), - GPA_ENUM_STRING_VAL(kGpaStatusErrorException, "GPA Error: Exception Occurred.")}; + GPA_ENUM_STRING_VAL(kGpaStatusErrorException, "GPA Error: C++ Exception Occurred.")}; /// Size of kErrorString array. static size_t kErrorStringSize = sizeof(kErrorString) / sizeof(const char*); From bd9f67d08c2da2aa8b7e3c7ad38142692fbf9f4b Mon Sep 17 00:00:00 2001 From: John Cortell Date: Wed, 15 Mar 2023 12:39:41 -0500 Subject: [PATCH 2/2] Fixed some accidental changes in previous commit --- .../examples/vulkan/vk_color_cube/vk_color_cube.cc | 8 ++++---- .../examples/vulkan/vk_color_cube/vk_color_cube.h | 13 ++++++++++--- source/gpu_perf_api_common/gpu_perf_api.cc | 13 +------------ 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/source/examples/vulkan/vk_color_cube/vk_color_cube.cc b/source/examples/vulkan/vk_color_cube/vk_color_cube.cc index 090ae242..5308e4d6 100644 --- a/source/examples/vulkan/vk_color_cube/vk_color_cube.cc +++ b/source/examples/vulkan/vk_color_cube/vk_color_cube.cc @@ -421,6 +421,7 @@ bool AMDVulkanDemo::InitializeGpa() if (kGpaStatusOk != status) { AMDVulkanDemoVkUtils::Log("ERROR: GpaGetFuncTable failed with status %d", status); + delete gpa_function_table; return false; } @@ -1814,10 +1815,9 @@ void AMDVulkanDemo::DrawScene() // This example only renders one set of profiles (aka, only the number of passes needed to generate one set of results). unsigned int profile_set = 0; - // NOTE: we can't loop over these because it is not guaranteed that the sample_ids will be 0-based and monotonically increasing. - gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, 0, print_debug_output_, Verify(), ConfirmSuccess()); - gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, 1, print_debug_output_, Verify(), ConfirmSuccess()); - gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, 2, print_debug_output_, Verify(), ConfirmSuccess()); + gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, AMDVulkanDemo::kGpaSampleIdCube, print_debug_output_, Verify(), ConfirmSuccess()); + gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, AMDVulkanDemo::kGpaSampleIdWireframe, print_debug_output_, Verify(), ConfirmSuccess()); + gpu_perf_api_helper_.PrintGpaSampleResults(gpa_context_id_, gpa_session_id_, profile_set, AMDVulkanDemo::kGpaSampleIdCubeAndWireframe, print_debug_output_, Verify(), ConfirmSuccess()); // Close the CSV file so that it actually gets saved out. gpu_perf_api_helper_.CloseCSVFile(); diff --git a/source/examples/vulkan/vk_color_cube/vk_color_cube.h b/source/examples/vulkan/vk_color_cube/vk_color_cube.h index 0fe8476e..30804a42 100644 --- a/source/examples/vulkan/vk_color_cube/vk_color_cube.h +++ b/source/examples/vulkan/vk_color_cube/vk_color_cube.h @@ -142,6 +142,13 @@ class AMDVulkanDemo : public gpa_example::GpaSampleApp /// Default window height. const uint32_t kDefaultWindowHeight = 300; + /// Note that GPA sample IDs are client defined. However, the Vulkan GPA + /// extension assigns an ID to each sample (they are not client defined). + /// The GPA library manages the mapping between them. These are the former. + static constexpr GpaUInt32 kGpaSampleIdCube = 0; + static constexpr GpaUInt32 kGpaSampleIdWireframe = 1; + static constexpr GpaUInt32 kGpaSampleIdCubeAndWireframe = 2; + #ifdef ANDROID inline void SetWindow(ANativeWindow* native_window) { @@ -242,7 +249,7 @@ class AMDVulkanDemo : public gpa_example::GpaSampleApp /// Sample Id that the application (not GPA) assigns to the cube. /// The cube will have this same sample_id in all passes. - const GpaUInt32 gpa_sample_id = 0; + const GpaUInt32 gpa_sample_id = kGpaSampleIdCube; } cube_; /// @brief Container for objects related to drawing the wireframe. @@ -256,7 +263,7 @@ class AMDVulkanDemo : public gpa_example::GpaSampleApp /// Sample Id that the application (not GPA) assigns to the wireframe. /// The wireframe will have this same sample_id in all passes. - const GpaUInt32 gpa_sample_id = 1; + const GpaUInt32 gpa_sample_id = kGpaSampleIdWireframe; } wire_frame_; /// @brief Container for objects related to drawing the cube and wireframe. @@ -279,7 +286,7 @@ class AMDVulkanDemo : public gpa_example::GpaSampleApp /// Sample Id that the application (not GPA) assigns to the cube wireframe. /// The combined cube + wireframe sample will have this same sample_id in all passes. - const GpaUInt32 gpa_sample_id = 2; + const GpaUInt32 gpa_sample_id = kGpaSampleIdCubeAndWireframe; } cube_and_wire_frame_; }; diff --git a/source/gpu_perf_api_common/gpu_perf_api.cc b/source/gpu_perf_api_common/gpu_perf_api.cc index 71c164be..115db237 100644 --- a/source/gpu_perf_api_common/gpu_perf_api.cc +++ b/source/gpu_perf_api_common/gpu_perf_api.cc @@ -47,7 +47,7 @@ extern IGpaImplementor* gpa_imp; ///< GPA implementor instance. } \ if (index >= num_counters) \ { \ - GPA_LOG_ERROR("Parameter index is %d but must be less than %d.", index, num_counters); \ + GPA_LOG_ERROR("Parameter %s is %d but must be less than %d.", #index, index, num_counters); \ return kGpaStatusErrorIndexOutOfRange; \ } @@ -493,15 +493,6 @@ GPA_LIB_DECL GpaStatus GpaGetDeviceGeneration(GpaContextId gpa_context_id, GpaHw case GDT_HW_GENERATION_GFX11: *hardware_generation = kGpaHwGenerationGfx11; break; - case GDT_HW_GENERATION_GFX104: - *hardware_generation = kGpaHwGenerationGfx104; - break; - case GDT_HW_GENERATION_GFX401: - *hardware_generation = kGpaHwGenerationGfx401; - break; - case GDT_HW_GENERATION_GFX402: - *hardware_generation = kGpaHwGenerationGfx402; - break; case GDT_HW_GENERATION_LAST: *hardware_generation = kGpaHwGenerationLast; break; @@ -856,8 +847,6 @@ GPA_LIB_DECL GpaStatus GpaDeleteSession(GpaSessionId gpa_session_id) GPA_LIB_DECL GpaStatus GpaBeginSession(GpaSessionId gpa_session_id) { - GPA_LOG_ERROR("jjjjjjjjjjjjj GpaBeginSession"); - try { PROFILE_FUNCTION(GpaBeginSession);