[GitHub] [tvm] masahi commented on a diff in pull request #10953: [Runtime][Vulkan] Add RGP support to TVM for vulkan device

[GitBox <gi...@apache.org>]

masahi commented on code in PR #10953:
URL: https://github.com/apache/tvm/pull/10953#discussion_r846844600


##########
src/runtime/vulkan/vulkan_instance.cc:
##########
@@ -59,6 +59,14 @@ VulkanInstance::VulkanInstance() {
     std::vector<const char*> required_extensions{};
     std::vector<const char*> optional_extensions{"VK_KHR_get_physical_device_properties2"};
 
+    // Check if RGP support is needed. If needed, enable VK_EXT_debug_utils extension for
+    // inserting debug labels into the queue.
+    const char* val = getenv("TVM_USE_AMD_RGP");

Review Comment:
   Use `BoolEnvironmentVar`
   https://github.com/apache/tvm/blob/ef7143e1fc1a12993f584ce37dbde38b75966b40/src/runtime/vulkan/vulkan_instance.cc#L36



##########
src/runtime/vulkan/vulkan_wrapped_func.cc:
##########
@@ -164,6 +164,15 @@ void VulkanWrappedFunc::operator()(TVMArgs args, TVMRetValue* rv,
     deferred_token.buffers_[i] = descriptor_buffers[i].buffer;
   }
   device.ThreadLocalStream().LaunchDeferred(deferred_initializer, deferred_kernel, deferred_token);
+
+  if (device.UseDebugUtilsLabel()) {
+    VkDebugUtilsLabelEXT dispatch_label = {VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT,
+                                           NULL,
+                                           func_name_.c_str(),
+                                           {0.0f, 0.0f, 0.0f, 0.0f}};
+    device.queue_insert_debug_utils_label_functions->vkQueueInsertDebugUtilsLabelEXT(
+        device.Queue(), &dispatch_label);
+  }

Review Comment:
   Can you add this to the immediate-mode path as well? https://github.com/apache/tvm/blob/f18d8d40bab7218a79392b56cf3d7cbcf6917e47/src/runtime/vulkan/vulkan_wrapped_func.cc#L73
   
   RADV supports push_descriptor so when we run on RADV, we are using that path. And RADV apparently has some support for RGP.



##########
src/runtime/vulkan/vulkan_stream.h:
##########
@@ -110,6 +125,7 @@ class VulkanStream {
   std::unordered_map<VkDescriptorSet, std::vector<VulkanStreamToken>> deferred_tokens_;
   std::vector<std::function<void(VulkanStreamState*)>> deferred_kernels_;
   VkCommandPool cmd_pool_;
+  VulkanStreamProfiler* profiler_;

Review Comment:
   I think this can live on the stack?



##########
src/runtime/vulkan/vulkan_stream.cc:
##########
@@ -55,11 +55,15 @@ VulkanStream::VulkanStream(const VulkanDevice* device)
   cb_begin.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT;
   cb_begin.pInheritanceInfo = 0;
   VULKAN_CALL(vkBeginCommandBuffer(state_->cmd_buffer_, &cb_begin));
+
+  profiler_ = new AmdRgpProfiler(device_);

Review Comment:
   Please do this only when RGP is enabled.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org