[PATCH] D78307: [mlir][vulkan-runner] Simplify vulkan launch call op.

Denis Khalikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 16 13:56:32 PDT 2020


denis13 marked an inline comment as done.
denis13 added inline comments.


================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertGPULaunchFuncToVulkanLaunchFunc.cpp:107
+                                         gpuLaunchTypes.begin() +
+                                             kVulkanLaunchNumConfigOperands);
+  vulkanLaunchTypes.append(gpuLaunchTypes.begin() +
----------------
mravishankar wrote:
> Super Nit : I think this usage is different from the comment on what `kVulkanLaunchNumConfigOperands` says. The comment implies, off the argument list to the vulkan call how many of the arguments relate to configuration. Here it is used in the context that the first 3 arguments of the arguments to the `gpu.launch_func` are needed for the vulkan launch. Had to read this a couple of times to understand. Maybe just add a comment that says that the first `kVulkanLaunchNumConfigOperands` of the `gpu.launch_func` op are the same as the config operands for the vulkan call.
Thanks for review! Sure, will fix it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78307





More information about the llvm-commits mailing list