[PATCH] D75531: [mlir][vulkan-runner] Add basic timing for compute pipeline

[Lei Zhang via Phabricator via llvm-commits]

antiagainst added inline comments.


================
Comment at: mlir/tools/mlir-vulkan-runner/VulkanRuntime.cpp:17
+#include <chrono>
+#include <iostream>
+
----------------
rriddle wrote:
> LLVM forbids the use of iostream.
> 
> https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden
Changed to llvm::outs. I saw we are using iostream already in RunnerUtils.h and cblas_interface.h so wanted to be consistent there. Should those be fixed?


================
Comment at: mlir/tools/mlir-vulkan-runner/VulkanRuntime.cpp:722
+    // Get anotehr timestamp after invoking the compute shader.
+    vkCmdWriteTimestamp(commandBuffer, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT,
+                        queryPool, 1);
----------------
benvanik wrote:
> you want to insert this after the vkCmdDispatch below - the timestamp only latches the value after previous commands complete (in this case, just the timestamp reset/initial query)
Thanks! I find the spec a bit difficult to parse: "latches the value of the timer when all previous commands have completed executing as far as the specified pipeline stage". I interpret it as the pipeline stage (`VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT`) controls when to write out the timestamp, not the placement of the `vkCmdWriteTimestamp` command. (Otherwise the pipeline stage specification does not really make sense.) But I do see there is a difference if I move the second `vkCmdWriteTimestamp` below `vkCmdDispatch`. So I'm a bit puzzled here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75531/new/

https://reviews.llvm.org/D75531