[PATCH] D76059: [mlir][vulkan-runner] Use C-compatible wrapper emission.
Lei Zhang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 16 18:35:50 PDT 2020
antiagainst added a comment.
Awesome! Thanks for the fix, Denis! Sorry for the delay. Overall LGTM; just a few nits. :)
================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp:118
+ /// op.
+ bool isCiFaceVulkanLaunchCallOp(LLVM::CallOp callOp) {
+ return (callOp.callee() &&
----------------
Nit: `isCInterfaceVulkanLaunchCallOp`? Just spell it out to make it clear. Applies to other parameter names, etc.
================
Comment at: mlir/lib/Conversion/GPUToVulkan/ConvertLaunchFuncToVulkanCalls.cpp:149
+ std::pair<StringAttr, StringAttr> spirvAttributes;
+ static constexpr const uint32_t kNumConfigOps = 6;
};
----------------
I think we can use `gpu::LaunchOp::kNumConfigOperands` instead of having our own here.
================
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) {
----------------
Nit: SPIR-V
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76059/new/
https://reviews.llvm.org/D76059
More information about the llvm-commits
mailing list