gemini-code-assist[bot] commented on code in PR #18672: URL: https://github.com/apache/tvm/pull/18672#discussion_r2703798908
########## src/runtime/vulkan/vulkan_timer.h: ########## @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#ifndef TVM_RUNTIME_VULKAN_VULKAN_TIMER_H_ +#define TVM_RUNTIME_VULKAN_VULKAN_TIMER_H_ + +#include <tvm/runtime/profiling.h> +#include <vulkan/vulkan.h> + +#include "vulkan_device.h" +#include "vulkan_stream.h" + +namespace tvm { +namespace runtime { +namespace vulkan { + +class VulkanDevice; + +/*! + * \brief Timer node for measuring GPU execution time using Vulkan. + * + * This class uses Vulkan timestamp queries to measure the time taken + * by GPU operations between `Start()` and `Stop()` calls. + */ +class VulkanTimerNode : public TimerNode { + public: + /*! + * \brief Constructs a VulkanTimerNode for the specified device. + * \param dev The TVM device to be used for timing. + */ + explicit VulkanTimerNode(Device dev); + + /*! + * \brief Destructor to clean up Vulkan resources. + */ + ~VulkanTimerNode() override; + + /*! + * \brief Starts the timer by recording a timestamp. + */ + void Start() override; + + /*! + * \brief Stops the timer by recording another timestamp. + */ + void Stop() override; + + /*! + * \brief Retrieves the elapsed time in nanoseconds. + * \return The elapsed time in nanoseconds between Start and Stop. + */ + int64_t SyncAndGetElapsedNanos() override; + + TVM_FFI_DECLARE_OBJECT_INFO_FINAL("runtime.opencl.VulkanTimerNode", VulkanTimerNode, TimerNode); Review Comment:  The FFI object type for `VulkanTimerNode` is incorrectly namespaced under `runtime.opencl`. It should be `runtime.vulkan`. This looks like a copy-paste error. ```suggestion TVM_FFI_DECLARE_OBJECT_INFO_FINAL("runtime.vulkan.VulkanTimerNode", VulkanTimerNode, TimerNode); ``` ########## src/runtime/vulkan/vulkan_stream.h: ########## @@ -117,6 +117,8 @@ class VulkanStream { // Synchronize the current stream `state_` with respect to the host. void Synchronize(); + VkCommandPool CommanPool() const { return cmd_pool_; } Review Comment:  There's a typo in the function name `CommanPool`. It should be `CommandPool`. ```suggestion VkCommandPool CommandPool() const { return cmd_pool_; } ``` ########## cmake/utils/FindVulkan.cmake: ########## @@ -46,45 +46,65 @@ macro(find_vulkan use_vulkan use_khronos_spirv) if(IS_DIRECTORY ${use_khronos_spirv}) set(__use_khronos_spirv ${use_khronos_spirv}) - message(STATUS "Custom khronos spirv PATH=" ${__use_khronos_spirv}) + message(STATUS "Using custom Khronos SPIRV path: ${__use_khronos_spirv}") else() set(__use_khronos_spirv "") endif() if(CMAKE_SYSTEM_NAME STREQUAL "Android") - set(VULKAN_NDK_SRC ${CMAKE_ANDROID_NDK}/sources/third_party/vulkan/src) - set(Vulkan_INCLUDE_DIRS ${VULKAN_NDK_SRC}/include) - set(Vulkan_FOUND TRUE) - message(STATUS "Android Vulkan_INCLUDE_DIRS=" ${Vulkan_INCLUDE_DIRS}) - message(STATUS "Skip finding SPIRV in Android, make sure you only build tvm runtime.") - return() - endif() + message(STATUS "Detected Android build") + + set(Vulkan_INCLUDE_DIRS "${CMAKE_SYSROOT}/usr/include/vulkan") + + # Map Android ABI to architecture + set(ANDROID_LIB_ARCH "") + if(CMAKE_ANDROID_ARCH_ABI STREQUAL "arm64-v8a") + set(ANDROID_LIB_ARCH "aarch64-linux-android") + elseif(CMAKE_ANDROID_ARCH_ABI STREQUAL "armeabi-v7a") + set(ANDROID_LIB_ARCH "arm-linux-androideabi") + elseif(CMAKE_ANDROID_ARCH_ABI STREQUAL "x86") + set(ANDROID_LIB_ARCH "i686-linux-android") + elseif(CMAKE_ANDROID_ARCH_ABI STREQUAL "x86_64") + set(ANDROID_LIB_ARCH "x86_64-linux-android") + else() + message(FATAL_ERROR "Unsupported Android ABI: ${CMAKE_ANDROID_ARCH_ABI}") + endif() + + # Find Vulkan library for Android + set(Vulkan_LIB_PATH "${CMAKE_SYSROOT}/usr/lib/${ANDROID_LIB_ARCH}/27") + find_library(Vulkan_LIBRARY NAMES vulkan libvulkan.so PATHS ${Vulkan_LIB_PATH} NO_DEFAULT_PATH) + + if(Vulkan_LIBRARY) + set(Vulkan_FOUND TRUE) + else() + message(FATAL_ERROR "Could not find Vulkan lib in ${Vulkan_LIB_PATH}") + endif() + + else() + message(STATUS "__vulkan_sdk:- " ${__vulkan_sdk}) Review Comment:  This `message` seems to be a leftover debug print. It should be removed to keep the build logs clean. ########## src/runtime/vulkan/vulkan_wrapped_func.cc: ########## @@ -101,7 +151,7 @@ void VulkanWrappedFunc::operator()(ffi::PackedArgs args, ffi::Any* rv, if (device.UseDebugUtilsLabel()) { VkDebugUtilsLabelEXT dispatch_label = {VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT, - nullptr, + NULL, Review Comment:  For consistency with modern C++ and the rest of the codebase, `nullptr` should be used instead of `NULL`. ```suggestion nullptr, ``` ########## src/runtime/vulkan/vulkan_wrapped_func.cc: ########## @@ -176,7 +235,7 @@ void VulkanWrappedFunc::operator()(ffi::PackedArgs args, ffi::Any* rv, if (device.UseDebugUtilsLabel()) { VkDebugUtilsLabelEXT dispatch_label = {VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT, - nullptr, + NULL, Review Comment:  For consistency with modern C++ and the rest of the codebase, `nullptr` should be used instead of `NULL`. ```suggestion nullptr, ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
