[PATCH] D76059: [mlir][vulkan-runner] Use C-compatible wrapper emission.

Tue Mar 17 03:44:06 PDT 2020

denis13 marked 3 inline comments as done and an inline comment as not done.
denis13 added inline comments.

================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp:118
+  /// op.
+  bool isCiFaceVulkanLaunchCallOp(LLVM::CallOp callOp) {
+    return (callOp.callee() &&
----------------
antiagainst wrote:
> Nit: `isCInterfaceVulkanLaunchCallOp`? Just spell it out to make it clear. Applies to other parameter names, etc.
Thanks, fixed!

================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp:149
+  std::pair<StringAttr, StringAttr> spirvAttributes;
+  static constexpr const uint32_t kNumConfigOps = 6;
 };
----------------
antiagainst wrote:
> I think we can use `gpu::LaunchOp::kNumConfigOperands` instead of having our own here.
Thanks for pointing on this! Fixed.

================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp:157
+
+  // Collect SPIRV attributes such as `spirv_blob` and `spirv_entry_point_name`.
   getModule().walk([this](LLVM::CallOp op) {
----------------
antiagainst wrote:
> Nit: SPIR-V
Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76059